| Message ID | 20190712001708.170259-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Luca Coelho |
| Headers | show |
| Series | [-next] iwlwifi: dbg: work around clang bug by marking debug strings static | expand |
On Thu, Jul 11, 2019 at 5:17 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > in function `_iwl_fw_dbg_apply_point': > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > when the following configs are enabled: > - CONFIG_IWLWIFI > - CONFIG_IWLMVM > - CONFIG_KASAN > > Work around the issue for now by marking the debug strings as `static`, > which they probably should be any ways. > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > Reported-by: Arnd Bergmann <arnd@arndb.de> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Sorry, I forgot a very important: Suggested-by: Eli Friedman <efriedma@quicinc.com> > --- > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > 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) { > -- > 2.22.0.410.gd8fdbe21b5-goog >
On Thu, Jul 11, 2019 at 05:17:06PM -0700, Nick Desaulniers wrote: > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > in function `_iwl_fw_dbg_apply_point': > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > when the following configs are enabled: > - CONFIG_IWLWIFI > - CONFIG_IWLMVM > - CONFIG_KASAN > > Work around the issue for now by marking the debug strings as `static`, > which they probably should be any ways. > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > Reported-by: Arnd Bergmann <arnd@arndb.de> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Applied on next-20190711 and I can confirm that this fixes the issue we observed. Thanks to you for wrapping up the patch and sending it and to Eli for giving the suggestion. Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote: > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > in function `_iwl_fw_dbg_apply_point': > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > when the following configs are enabled: > - CONFIG_IWLWIFI > - CONFIG_IWLMVM > - CONFIG_KASAN > > Work around the issue for now by marking the debug strings as `static`, > which they probably should be any ways. > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > Reported-by: Arnd Bergmann <arnd@arndb.de> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > 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"; Better still would be to use the format string directly in both locations instead of trying to deduplicate it via storing it into a separate pointer. Let the compiler/linker consolidate the format. It's smaller object code, allows format/argument verification, and is simpler for humans to understand. --- diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c index e411ac98290d..25e6712932b8 100644 --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c @@ -2438,17 +2438,17 @@ 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"; if (img_name_len != IWL_FW_INI_MAX_IMG_NAME_LEN) { - IWL_WARN(fwrt, err_str, ext, "image", img_name_len, + IWL_WARN(fwrt, "WRT: ext=%d. Invalid %s name length %d, expected %d\n", + 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, "WRT: ext=%d. Invalid %s name length %d, expected %d\n", + ext, "debug cfg", dbg_cfg_name_len, IWL_FW_INI_MAX_DBG_CFG_NAME_LEN); return; } @@ -2775,8 +2775,6 @@ 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"; switch (type) { case IWL_UCODE_TLV_TYPE_DEBUG_INFO: @@ -2786,8 +2784,8 @@ 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, - "buffer allocation"); + IWL_ERR(fwrt, "WRT: ext=%d. Invalid apply point %d for %s\n", + ext, pnt, "buffer allocation"); goto next; } @@ -2797,8 +2795,8 @@ 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, - "host command"); + IWL_ERR(fwrt, "WRT: ext=%d. Invalid apply point %d for %s\n", + ext, pnt, "host command"); goto next; } iwl_fw_dbg_send_hcmd(fwrt, tlv, ext);
On Thu, Jul 11, 2019 at 7:15 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote: > > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > > in function `_iwl_fw_dbg_apply_point': > > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > > > when the following configs are enabled: > > - CONFIG_IWLWIFI > > - CONFIG_IWLMVM > > - CONFIG_KASAN > > > > Work around the issue for now by marking the debug strings as `static`, > > which they probably should be any ways. > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > 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"; > > Better still would be to use the format string directly > in both locations instead of trying to deduplicate it > via storing it into a separate pointer. > > Let the compiler/linker consolidate the format. > It's smaller object code, allows format/argument verification, > and is simpler for humans to understand. Whichever Kalle prefers, I just want my CI green again. > > --- > diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c > index e411ac98290d..25e6712932b8 100644 > --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c > +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c > @@ -2438,17 +2438,17 @@ 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"; > > if (img_name_len != IWL_FW_INI_MAX_IMG_NAME_LEN) { > - IWL_WARN(fwrt, err_str, ext, "image", img_name_len, > + IWL_WARN(fwrt, "WRT: ext=%d. Invalid %s name length %d, expected %d\n", > + 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, "WRT: ext=%d. Invalid %s name length %d, expected %d\n", > + ext, "debug cfg", dbg_cfg_name_len, > IWL_FW_INI_MAX_DBG_CFG_NAME_LEN); > return; > } > @@ -2775,8 +2775,6 @@ 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"; > > switch (type) { > case IWL_UCODE_TLV_TYPE_DEBUG_INFO: > @@ -2786,8 +2784,8 @@ 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, > - "buffer allocation"); > + IWL_ERR(fwrt, "WRT: ext=%d. Invalid apply point %d for %s\n", > + ext, pnt, "buffer allocation"); > goto next; > } > > @@ -2797,8 +2795,8 @@ 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, > - "host command"); > + IWL_ERR(fwrt, "WRT: ext=%d. Invalid apply point %d for %s\n", > + ext, pnt, "host command"); > goto next; > } > iwl_fw_dbg_send_hcmd(fwrt, tlv, ext); > >
On Tue, 2019-07-16 at 10:28 -0700, Nick Desaulniers wrote: > On Thu, Jul 11, 2019 at 7:15 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote: > > > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > > > > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > > > in function `_iwl_fw_dbg_apply_point': > > > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > > > > > when the following configs are enabled: > > > - CONFIG_IWLWIFI > > > - CONFIG_IWLMVM > > > - CONFIG_KASAN > > > > > > Work around the issue for now by marking the debug strings as `static`, > > > which they probably should be any ways. > > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > > > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > 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"; > > > > Better still would be to use the format string directly > > in both locations instead of trying to deduplicate it > > via storing it into a separate pointer. > > > > Let the compiler/linker consolidate the format. > > It's smaller object code, allows format/argument verification, > > and is simpler for humans to understand. > > Whichever Kalle prefers, I just want my CI green again. We already changed this in a later internal patch, which will reach the mainline (-next) soon. So let's skip this for now. -- Cheers, Luca.
On Tue, Jul 16, 2019 at 2:15 PM Luca Coelho <luca@coelho.fi> wrote: > > On Tue, 2019-07-16 at 10:28 -0700, Nick Desaulniers wrote: > > On Thu, Jul 11, 2019 at 7:15 PM Joe Perches <joe@perches.com> wrote: > > > Better still would be to use the format string directly > > > in both locations instead of trying to deduplicate it > > > via storing it into a separate pointer. > > > > > > Let the compiler/linker consolidate the format. > > > It's smaller object code, allows format/argument verification, > > > and is simpler for humans to understand. > > > > Whichever Kalle prefers, I just want my CI green again. > > We already changed this in a later internal patch, which will reach the > mainline (-next) soon. So let's skip this for now. Ok, but this has now regressed into mainline and is blocking Linaro's ToolChain Working Group's CI, so if you could send a bugfix ASAP we'd appreciate it.
On Tue, Jul 16, 2019 at 11:15 PM Luca Coelho <luca@coelho.fi> wrote: > > On Tue, 2019-07-16 at 10:28 -0700, Nick Desaulniers wrote: > > On Thu, Jul 11, 2019 at 7:15 PM Joe Perches <joe@perches.com> wrote: > > > On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote: > > > > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > > > > > > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > > > > in function `_iwl_fw_dbg_apply_point': > > > > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' > > > > > > > > when the following configs are enabled: > > > > - CONFIG_IWLWIFI > > > > - CONFIG_IWLMVM > > > > - CONFIG_KASAN > > > > > > > > Work around the issue for now by marking the debug strings as `static`, > > > > which they probably should be any ways. > > > > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580 > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/580 > > > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > > > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > > > --- > > > > drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > 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"; > > > > > > Better still would be to use the format string directly > > > in both locations instead of trying to deduplicate it > > > via storing it into a separate pointer. > > > > > > Let the compiler/linker consolidate the format. > > > It's smaller object code, allows format/argument verification, > > > and is simpler for humans to understand. > > > > Whichever Kalle prefers, I just want my CI green again. > > We already changed this in a later internal patch, which will reach the > mainline (-next) soon. So let's skip this for now. > Do you have a link to your internal Git or patchwork for this? I am interested in testing it. - Sedat -
Nick Desaulniers <ndesaulniers@google.com> writes: > Commit r353569 in prerelease Clang-9 is producing a linkage failure: > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: > in function `_iwl_fw_dbg_apply_point': > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' This breakage is also seen in older GCC versions (v4.6.3 at least): drivers/net/wireless/intel/iwlwifi/fw/dbg.c: In function 'iwl_fw_dbg_info_apply.isra.10': drivers/net/wireless/intel/iwlwifi/fw/dbg.c:2445:3: error: call to '__compiletime_assert_2446' declared with attribute error: BUILD_BUG_ON failed: err_str[sizeof(err_str) - 2] != '\n' drivers/net/wireless/intel/iwlwifi/fw/dbg.c:2451:3: error: call to '__compiletime_assert_2452' declared with attribute error: BUILD_BUG_ON failed: err_str[sizeof(err_str) - 2] != '\n' drivers/net/wireless/intel/iwlwifi/fw/dbg.c: In function '_iwl_fw_dbg_apply_point': drivers/net/wireless/intel/iwlwifi/fw/dbg.c:2789:5: error: call to '__compiletime_assert_2790' declared with attribute error: BUILD_BUG_ON failed: invalid_ap_str[sizeof(invalid_ap_str) - 2] != '\n' drivers/net/wireless/intel/iwlwifi/fw/dbg.c:2800:5: error: call to '__compiletime_assert_2801' declared with attribute error: BUILD_BUG_ON failed: invalid_ap_str[sizeof(invalid_ap_str) - 2] != '\n' http://kisskb.ellerman.id.au/kisskb/buildresult/13902155/ Luca, you said this was already fixed in your internal tree, and the fix would appear soon in next, but I don't see anything in linux-next? cheers
> Luca, you said this was already fixed in your internal tree, and the fix > would appear soon in next, but I don't see anything in linux-next? Luca is still on vacation, but I just sent out a version of the patch we had applied internally. Also turns out it wasn't actually _fixed_, just _moved_, so those internal patches wouldn't have helped anyway. johannes
Johannes Berg <johannes@sipsolutions.net> writes: >> Luca, you said this was already fixed in your internal tree, and the fix >> would appear soon in next, but I don't see anything in linux-next? > > Luca is still on vacation, but I just sent out a version of the patch we > had applied internally. Awesome, thanks. cheers
On Thu, Aug 1, 2019 at 12:11 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > Luca, you said this was already fixed in your internal tree, and the fix > > would appear soon in next, but I don't see anything in linux-next? > > Luca is still on vacation, but I just sent out a version of the patch we > had applied internally. > > Also turns out it wasn't actually _fixed_, just _moved_, so those > internal patches wouldn't have helped anyway. Thanks for the report. Do you have a link? I'll rebase my patch then.
On Tue, Aug 06, 2019 at 03:37:42PM -0700, Nick Desaulniers wrote: > On Thu, Aug 1, 2019 at 12:11 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > > > > Luca, you said this was already fixed in your internal tree, and the fix > > > would appear soon in next, but I don't see anything in linux-next? > > > > Luca is still on vacation, but I just sent out a version of the patch we > > had applied internally. > > > > Also turns out it wasn't actually _fixed_, just _moved_, so those > > internal patches wouldn't have helped anyway. > > Thanks for the report. Do you have a link? > I'll rebase my patch then. > -- > Thanks, > ~Nick Desaulniers Just for everyone else (since I commented on our issue tracker), this is now fixed in Linus's tree as of commit 1f6607250331 ("iwlwifi: dbg_ini: fix compile time assert build errors"). Cheers, Nathan
On Tue, 2019-08-06 at 22:15 -0700, Nathan Chancellor wrote: > On Tue, Aug 06, 2019 at 03:37:42PM -0700, Nick Desaulniers wrote: > > On Thu, Aug 1, 2019 at 12:11 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > > > > > > Luca, you said this was already fixed in your internal tree, and the fix > > > > would appear soon in next, but I don't see anything in linux-next? > > > > > > Luca is still on vacation, but I just sent out a version of the patch we > > > had applied internally. > > > > > > Also turns out it wasn't actually _fixed_, just _moved_, so those > > > internal patches wouldn't have helped anyway. > > > > Thanks for the report. Do you have a link? > > I'll rebase my patch then. > > -- > > Thanks, > > ~Nick Desaulniers > > Just for everyone else (since I commented on our issue tracker), this is > now fixed in Linus's tree as of commit 1f6607250331 ("iwlwifi: dbg_ini: > fix compile time assert build errors"). Yes, thanks Nathan! I was just digging for this patch to reply to you, I'm still catching up with what happened during my vacations. -- Cheers, Luca.
On Tue, 2019-08-06 at 22:15 -0700, Nathan Chancellor wrote: > Just for everyone else (since I commented on our issue tracker), this is > now fixed in Linus's tree as of commit 1f6607250331 ("iwlwifi: dbg_ini: > fix compile time assert build errors"). I think this change is incomplete and suggest you add this to remove the use of another const char * format. --- drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c index 4d81776f576d..6b15e2e8cd37 100644 --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c @@ -2593,20 +2593,20 @@ static void iwl_fw_dbg_update_regions(struct iwl_fw_runtime *fwrt, { void *iter = (void *)tlv->region_config; int i, size = le32_to_cpu(tlv->num_regions); - const char *err_st = - "WRT: ext=%d. Invalid region %s %d for apply point %d\n"; for (i = 0; i < size; i++) { struct iwl_fw_ini_region_cfg *reg = iter, **active; int id = le32_to_cpu(reg->region_id); u32 type = le32_to_cpu(reg->region_type); - if (WARN(id >= ARRAY_SIZE(fwrt->dump.active_regs), err_st, ext, - "id", id, pnt)) + if (WARN(id >= ARRAY_SIZE(fwrt->dump.active_regs), + "WRT: ext=%d. Invalid region id %d for apply point %d\n", + ext, id, pnt)) break; - if (WARN(type == 0 || type >= IWL_FW_INI_REGION_NUM, err_st, - ext, "type", type, pnt)) + if (WARN(type == 0 || type >= IWL_FW_INI_REGION_NUM, + "WRT: ext=%d. Invalid region type %d for apply point %d\n", + ext, type, pnt)) break; active = &fwrt->dump.active_regs[id];
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) {
Commit r353569 in prerelease Clang-9 is producing a linkage failure: ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o: in function `_iwl_fw_dbg_apply_point': dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387' when the following configs are enabled: - CONFIG_IWLWIFI - CONFIG_IWLMVM - CONFIG_KASAN Work around the issue for now by marking the debug strings as `static`, which they probably should be any ways. Link: https://bugs.llvm.org/show_bug.cgi?id=42580 Link: https://github.com/ClangBuiltLinux/linux/issues/580 Reported-by: Arnd Bergmann <arnd@arndb.de> Reported-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)