-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
__compiletime_assert in drivers/net/wireless/intel/iwlwifi/fw/dbg.c #580
Comments
Probably a failed |
Ahhh, it's in the
/* not all compilers can evaluate strlen() at compile time, so use sizeof() */
#define CHECK_FOR_NEWLINE(f) BUILD_BUG_ON(f[sizeof(f) - 2] != '\n')
/* No matter what is m (priv, bus, trans), this will work */
#define IWL_ERR_DEV(d, f, a...) \
do { \
CHECK_FOR_NEWLINE(f); \
__iwl_err((d), false, false, f, ## a); \
} while (0)
#define IWL_ERR(m, f, a...) \
IWL_ERR_DEV((m)->dev, f, ## a)
#define IWL_WARN(m, f, a...) \
do { \
CHECK_FOR_NEWLINE(f); \
__iwl_warn((m)->dev, f, ## a); \
} while (0)
#define IWL_INFO(m, f, a...) \
do { \
CHECK_FOR_NEWLINE(f); \
__iwl_info((m)->dev, f, ## a); \
} while (0)
#define IWL_CRIT(m, f, a...) \
do { \
CHECK_FOR_NEWLINE(f); \
__iwl_crit((m)->dev, f, ## a); \
} while (0)
static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
struct iwl_fw_ini_debug_info_tlv *dbg_info,
bool ext, enum iwl_fw_ini_apply_point pnt)
{
u32 img_name_len = le32_to_cpu(dbg_info->img_name_len);
u32 dbg_cfg_name_len = le32_to_cpu(dbg_info->dbg_cfg_name_len);
const char err_str[] =
"WRT: ext=%d. Invalid %s name length %d, expected %d\n";
if (img_name_len != IWL_FW_INI_MAX_IMG_NAME_LEN) {
IWL_WARN(fwrt, err_str, ext, "image", img_name_len,
IWL_FW_INI_MAX_IMG_NAME_LEN);
return;
}
if (dbg_cfg_name_len != IWL_FW_INI_MAX_DBG_CFG_NAME_LEN) {
IWL_WARN(fwrt, err_str, ext, "debug cfg", dbg_cfg_name_len,
IWL_FW_INI_MAX_DBG_CFG_NAME_LEN);
return;
}
if (ext) {
memcpy(fwrt->dump.external_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.external_dbg_cfg_name));
} else {
memcpy(fwrt->dump.img_name, dbg_info->img_name,
sizeof(fwrt->dump.img_name));
memcpy(fwrt->dump.internal_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.internal_dbg_cfg_name));
}
} Preprocessed source: static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
struct iwl_fw_ini_debug_info_tlv *dbg_info,
bool ext, enum iwl_fw_ini_apply_point pnt)
{
u32 img_name_len = (( __u32)(__le32)(dbg_info->img_name_len));
u32 dbg_cfg_name_len = (( __u32)(__le32)(dbg_info->dbg_cfg_name_len));
const char err_str[] =
"WRT: ext=%d. Invalid %s name length %d, expected %d\n";
if (img_name_len != 32) {
do { do { extern void __compiletime_assert_2446(void) ; if (!(!(err_str[sizeof(err_str) - 2] != '\n'))) __compiletime_assert_2446(); } while (0); __iwl_warn((fwrt)->dev, err_str, ext, "image", img_name_len, 32); } while (0);
return;
}
if (dbg_cfg_name_len != 64) {
do { do { extern void __compiletime_assert_2452(void) ; if (!(!(err_str[sizeof(err_str) - 2] != '\n'))) __compiletime_assert_2452(); } while (0); __iwl_warn((fwrt)->dev, err_str, ext, "debug cfg", dbg_cfg_name_len, 64); } while (0);
return;
}
if (ext) {
memcpy(fwrt->dump.external_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.external_dbg_cfg_name));
} else {
memcpy(fwrt->dump.img_name, dbg_info->img_name,
sizeof(fwrt->dump.img_name));
memcpy(fwrt->dump.internal_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.internal_dbg_cfg_name));
}
} compared to if (!fwrt->dump.d3_debug_data) {
fwrt->dump.d3_debug_data = kmalloc(cfg->d3_debug_data_length,
((( gfp_t)(0x400u|0x800u)) | (( gfp_t)0x40u) | (( gfp_t)0x80u)));
if (!fwrt->dump.d3_debug_data) {
do { do { extern void __compiletime_assert_2423(void) ; if (!(!("failed to allocate memory for D3 debug data\n"[sizeof("failed to allocate memory for D3 debug data\n") - 2] != '\n'))) __compiletime_assert_2423(); } while (0); __iwl_err(((fwrt)->dev), false, false, "failed to allocate memory for D3 debug data\n"); } while (0);
return;
}
} There is another occurrence of this pattern in a later commit ( Is there a clean way to resolve this other than eliminating the use of the problematic diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index e411ac98290d..b2b2e6d20bf9 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -2438,20 +2438,20 @@ static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
{
u32 img_name_len = le32_to_cpu(dbg_info->img_name_len);
u32 dbg_cfg_name_len = le32_to_cpu(dbg_info->dbg_cfg_name_len);
- const char err_str[] =
- "WRT: ext=%d. Invalid %s name length %d, expected %d\n";
+#define ERR_STR "WRT: ext=%d. Invalid %s name length %d, expected %d\n"
if (img_name_len != IWL_FW_INI_MAX_IMG_NAME_LEN) {
- IWL_WARN(fwrt, err_str, ext, "image", img_name_len,
+ IWL_WARN(fwrt, ERR_STR, ext, "image", img_name_len,
IWL_FW_INI_MAX_IMG_NAME_LEN);
return;
}
if (dbg_cfg_name_len != IWL_FW_INI_MAX_DBG_CFG_NAME_LEN) {
- IWL_WARN(fwrt, err_str, ext, "debug cfg", dbg_cfg_name_len,
+ IWL_WARN(fwrt, ERR_STR, ext, "debug cfg", dbg_cfg_name_len,
IWL_FW_INI_MAX_DBG_CFG_NAME_LEN);
return;
}
+#undef ERR_STR
if (ext) {
memcpy(fwrt->dump.external_dbg_cfg_name, dbg_info->dbg_cfg_name,
@@ -2775,8 +2775,7 @@ static void _iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
struct iwl_ucode_tlv *tlv = iter;
void *ini_tlv = (void *)tlv->data;
u32 type = le32_to_cpu(tlv->type);
- const char invalid_ap_str[] =
- "WRT: ext=%d. Invalid apply point %d for %s\n";
+#define INVALID_AP_STR "WRT: ext=%d. Invalid apply point %d for %s\n"
switch (type) {
case IWL_UCODE_TLV_TYPE_DEBUG_INFO:
@@ -2786,7 +2785,7 @@ static void _iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
struct iwl_fw_ini_allocation_data *buf_alloc = ini_tlv;
if (pnt != IWL_FW_INI_APPLY_EARLY) {
- IWL_ERR(fwrt, invalid_ap_str, ext, pnt,
+ IWL_ERR(fwrt, INVALID_AP_STR, ext, pnt,
"buffer allocation");
goto next;
}
@@ -2797,10 +2796,11 @@ static void _iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
}
case IWL_UCODE_TLV_TYPE_HCMD:
if (pnt < IWL_FW_INI_APPLY_AFTER_ALIVE) {
- IWL_ERR(fwrt, invalid_ap_str, ext, pnt,
+ IWL_ERR(fwrt, INVALID_AP_STR, ext, pnt,
"host command");
goto next;
}
+#undef INVALID_AP_STR
iwl_fw_dbg_send_hcmd(fwrt, tlv, ext);
break;
case IWL_UCODE_TLV_TYPE_REGIONS: |
Something else might be up here as I cannot get this to trigger with x86_64_defconfig + all of the IWLWIFI configs:
I diffed the build flags but nothing immediately stands out: defconfig: allyesconfig: |
For those unreadable macro expansions: dump them in a file then run I'm guessing that the |
Thank you for the tip :)
Well, I think they are supposed to be generated because they are https://github.com/torvalds/linux/blob/master/include/linux/build_bug.h#L50 https://github.com/torvalds/linux/blob/master/include/linux/compiler.h#L345 I guess on the surface what is happening is clang is not able to resolve
I will take a look at this first thing tomorrow morning. |
Preprocessed source + Clang: static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
struct iwl_fw_ini_debug_info_tlv *dbg_info,
bool ext, enum iwl_fw_ini_apply_point pnt)
{
u32 img_name_len = ((__u32)(__le32)(dbg_info->img_name_len));
u32 dbg_cfg_name_len = ((__u32)(__le32)(dbg_info->dbg_cfg_name_len));
const char err_str[] =
"WRT: ext=%d. Invalid %s name length %d, expected %d\n";
if (img_name_len != 32) {
do {
do {
extern void __compiletime_assert_2446(void);
if (!(!(err_str[sizeof(err_str) - 2] != '\n')))
__compiletime_assert_2446();
} while (0);
__iwl_warn((fwrt)->dev, err_str, ext, "image",
img_name_len, 32);
} while (0);
return;
}
if (dbg_cfg_name_len != 64) {
do {
do {
extern void __compiletime_assert_2452(void);
if (!(!(err_str[sizeof(err_str) - 2] != '\n')))
__compiletime_assert_2452();
} while (0);
__iwl_warn((fwrt)->dev, err_str, ext, "debug cfg",
dbg_cfg_name_len, 64);
} while (0);
return;
}
if (ext) {
memcpy(fwrt->dump.external_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.external_dbg_cfg_name));
} else {
memcpy(fwrt->dump.img_name, dbg_info->img_name,
sizeof(fwrt->dump.img_name));
memcpy(fwrt->dump.internal_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.internal_dbg_cfg_name));
}
} GCC: static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
struct iwl_fw_ini_debug_info_tlv *dbg_info,
bool ext, enum iwl_fw_ini_apply_point pnt)
{
u32 img_name_len = ((__u32)(__le32)(dbg_info->img_name_len));
u32 dbg_cfg_name_len = ((__u32)(__le32)(dbg_info->dbg_cfg_name_len));
const char err_str[] =
"WRT: ext=%d. Invalid %s name length %d, expected %d\n";
if (img_name_len != 32) {
do {
do {
extern void
__compiletime_assert_2445(void) __attribute__((__error__(
"BUILD_BUG_ON failed: "
"err_str[sizeof(err_str) - 2] != '\\n'")));
if (!(!(err_str[sizeof(err_str) - 2] != '\n')))
__compiletime_assert_2445();
} while (0);
__iwl_warn((fwrt)->dev, err_str, ext, "image",
img_name_len, 32);
} while (0);
return;
}
if (dbg_cfg_name_len != 64) {
do {
do {
extern void
__compiletime_assert_2451(void) __attribute__((__error__(
"BUILD_BUG_ON failed: "
"err_str[sizeof(err_str) - 2] != '\\n'")));
if (!(!(err_str[sizeof(err_str) - 2] != '\n')))
__compiletime_assert_2451();
} while (0);
__iwl_warn((fwrt)->dev, err_str, ext, "debug cfg",
dbg_cfg_name_len, 64);
} while (0);
return;
}
if (ext) {
memcpy(fwrt->dump.external_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.external_dbg_cfg_name));
} else {
memcpy(fwrt->dump.img_name, dbg_info->img_name,
sizeof(fwrt->dump.img_name));
memcpy(fwrt->dump.internal_dbg_cfg_name, dbg_info->dbg_cfg_name,
sizeof(fwrt->dump.internal_dbg_cfg_name));
}
} The only real difference I see is the addition of the message in the compiletime_assert string, which originates from https://github.com/torvalds/linux/blob/v5.2-rc7/include/linux/compiler-gcc.h#L76 and https://github.com/torvalds/linux/blob/v5.2-rc7/include/linux/compiler.h#L324. |
Wasn't able to come up with a short reproducer: https://godbolt.org/z/tR9v22 Also, is that the correct snippet of preprocessed source? The number/counter suffix on those |
I tried Yes, it is the right snippet. I don't think I used the same revision between runs. |
I have tried getting
Does the interestingness test need to be improved or is creduce just not going to work for this? |
Figured out what I was doing wrong: the
I am attempting to properly reduce now. |
Turns out this is somehow related to
a;
b() {
char c[] = "WRT ext=%d. Invalid %s name length %d expected %d\n";
if (a)
if (c[sizeof(c) - 2] != '\n')
__compiletime_assert_2446__iwl_warn(c);
} with the following interestingness test: #!/bin/bash
gcc -O2 -c -o dbg-gcc.o dbg.i && \
! nm dbg-gcc.o | grep __compiletime_assert_2446 && \
/home/nathan/cbl/usr/bin/clang -O2 -no-integrated-as -c -o dbg-clang.o dbg.i && \
! nm dbg-clang.o | grep __compiletime_assert_2446 && \
/home/nathan/cbl/usr/bin/clang -O2 -no-integrated-as -fsanitize=kernel-address -c -o dbg-clang.o dbg.i && \
nm dbg-clang.o | grep __compiletime_assert_2446 The code generation massively changes with |
I feel like #define CHECK_FOR_NEWLINE(f) BUILD_BUG_ON(f[sizeof(f) - 2] != '\n')
/* No matter what is m (priv, bus, trans), this will work */
#define IWL_ERR_DEV(d, f, a...) \
do { \
CHECK_FOR_NEWLINE(f); \
__iwl_err((d), false, false, f, ## a); \
} while (0) should be replaced with: #define IWL_ERR_DEV(d, f, a...) \
do { \
__iwl_err((d), false, false, f "\n", ## a); \
} while (0) That way the preprocessor just always appends a new line to the format string. Then callers never need to remember to add a newline, and there's no non-portable obscure linkage failure. And worst case, there's two newlines which is no big deal. cc @gburgessiv |
I don't think that is a terrible idea. It might be worth sending along as an RFC to the iwlwifi maintainers just to gauge their reactions. However, I think it is rather interesting that KASAN prevents clang from being able to figure out that that string does end with a newline character and that should still probably be looked into. |
Quoting @gburgessiv :
|
That makes sense then, worth bringing up to the maintainers and see how they react. |
@arndb filed an upstream bug: https://llvm.org/pr42580 |
I bisected using Arnd's reproducer: https://llvm.org/pr42580 |
@nathanchance can you provide steps to reproduce? I cannot repro with: x86 defconfig + CONFIG_IWLWIFI + CONFIG_KASAN
:( |
I can reproduce with x86_64_defconfig +
For my bisect, I used Arnd's test case and the following script:
|
Eli mentions on the LLVM bug that marking |
CONFIG_IWLMVM is needed to compile drivers/net/wireless/intel/iwlwifi/fw/dbg.o. Then I can repro. |
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index e411ac98290d..f8c90ea4e9b4 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -2438,7 +2438,7 @@ static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
{
u32 img_name_len = le32_to_cpu(dbg_info->img_name_len);
u32 dbg_cfg_name_len = le32_to_cpu(dbg_info->dbg_cfg_name_len);
- const char err_str[] =
+ static const char err_str[] =
"WRT: ext=%d. Invalid %s name length %d, expected %d\n";
if (img_name_len != IWL_FW_INI_MAX_IMG_NAME_LEN) {
@@ -2775,7 +2775,7 @@ static void _iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
struct iwl_ucode_tlv *tlv = iter;
void *ini_tlv = (void *)tlv->data;
u32 type = le32_to_cpu(tlv->type);
- const char invalid_ap_str[] =
+ static const char invalid_ap_str[] =
"WRT: ext=%d. Invalid apply point %d for %s\n";
switch (type) { is a fix. Packaging it up now. |
This was fixed by an internal Intel patch (GCC 4.6.3 errors finally brought it out...): https://git.kernel.org/kvalo/wireless-drivers/c/1f66072503316134873060b24b7895dbbcccf00e It is now in -next, hopefully will be in 5.3-rc3 if we are lucky. |
|
Bisect log:
Bad commit: https://git.kernel.org/next/linux-next/c/57d88b116175cd6e9293bef5355094c7dab4b747
Nothing immediate sticks out but I'll try to look into it further later.
Quick reproducer:
make -j$(nproc) CC=clang allyesconfig drivers/net/wireless/intel/iwlwifi/ && nm drivers/net/wireless/intel/iwlwifi/fw/dbg.o | grep __compiletime_assert
The text was updated successfully, but these errors were encountered: