linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] coresight: etm4x: work around clang-12+ build failure
@ 2021-02-25  9:42 Arnd Bergmann
  2021-02-25 16:45 ` Mathieu Poirier
  2021-02-25 17:20 ` Suzuki K Poulose
  0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2021-02-25  9:42 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin,
	Nathan Chancellor, Nick Desaulniers
  Cc: Arnd Bergmann, Mike Leach, Leo Yan, Greg Kroah-Hartman,
	Sai Prakash Ranjan, coresight, linux-arm-kernel, linux-kernel,
	clang-built-linux

From: Arnd Bergmann <arnd@arndb.de>

clang-12 fails to build the etm4x driver with -fsanitize=array-bounds:

<instantiation>:1:7: error: expected constant expression in '.inst' directive
.inst (0xd5200000|((((2) << 19) | ((1) << 16) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 7) & 0x7)) << 12) | ((((((((((0x160 + (i * 4))))) >> 2))) & 0xf)) << 8) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 4) & 0x7)) << 5)))|(.L__reg_num_x8))
      ^
drivers/hwtracing/coresight/coresight-etm4x-core.c:702:4: note: while in macro instantiation
                        etm4x_relaxed_read32(csa, TRCCNTVRn(i));
                        ^
drivers/hwtracing/coresight/coresight-etm4x.h:403:4: note: expanded from macro 'etm4x_relaxed_read32'
                 read_etm4x_sysreg_offset((offset), false)))
                 ^
drivers/hwtracing/coresight/coresight-etm4x.h:383:12: note: expanded from macro 'read_etm4x_sysreg_offset'
                        __val = read_etm4x_sysreg_const_offset((offset));       \
                                ^
drivers/hwtracing/coresight/coresight-etm4x.h:149:2: note: expanded from macro 'read_etm4x_sysreg_const_offset'
        READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))
        ^
drivers/hwtracing/coresight/coresight-etm4x.h:144:2: note: expanded from macro 'READ_ETM4x_REG'
        read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG((reg)))
        ^
arch/arm64/include/asm/sysreg.h:1108:15: note: expanded from macro 'read_sysreg_s'
        asm volatile(__mrs_s("%0", r) : "=r" (__val));                  \
                     ^
arch/arm64/include/asm/sysreg.h:1074:2: note: expanded from macro '__mrs_s'
"       mrs_s " v ", " __stringify(r) "\n"                      \
 ^

It appears that the __builin_constant_p() check in
read_etm4x_sysreg_offset() falsely returns 'true' here because clang
decides finds that an out-of-bounds access to config->cntr_val[] cannot
happen, and then it unrolls the loop with constant register numbers. Then
when actually emitting the output, it fails to figure out the value again.

While this is incorrect behavior in clang, it is easy to work around
by avoiding the out-of-bounds array access. Do this by limiting the
loop counter to the actual dimension of the array.

Link: https://github.com/ClangBuiltLinux/linux/issues/1310
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 15016f757828..4cccf874a602 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info)
 			"timeout while waiting for PM stable Trace Status\n");
 
 	/* read the status of the single shot comparators */
-	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
+	for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) {
 		config->ss_status[i] =
 			etm4x_relaxed_read32(csa, TRCSSCSRn(i));
 	}
 
 	/* read back the current counter values */
-	for (i = 0; i < drvdata->nr_cntr; i++) {
+	for (i = 0; i < min_t(u32, drvdata->nr_cntr, ETMv4_MAX_CNTR); i++) {
 		config->cntr_val[i] =
 			etm4x_relaxed_read32(csa, TRCCNTVRn(i));
 	}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] coresight: etm4x: work around clang-12+ build failure
  2021-02-25  9:42 [PATCH] coresight: etm4x: work around clang-12+ build failure Arnd Bergmann
@ 2021-02-25 16:45 ` Mathieu Poirier
  2021-02-25 20:08   ` Arnd Bergmann
  2021-02-25 21:23   ` Nick Desaulniers
  2021-02-25 17:20 ` Suzuki K Poulose
  1 sibling, 2 replies; 6+ messages in thread
From: Mathieu Poirier @ 2021-02-25 16:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Suzuki K Poulose, Alexander Shishkin, Nathan Chancellor,
	Nick Desaulniers, Arnd Bergmann, Mike Leach, Leo Yan,
	Greg Kroah-Hartman, Sai Prakash Ranjan, coresight,
	linux-arm-kernel, linux-kernel, clang-built-linux

Good morning,

On Thu, Feb 25, 2021 at 10:42:58AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang-12 fails to build the etm4x driver with -fsanitize=array-bounds:
> 
> <instantiation>:1:7: error: expected constant expression in '.inst' directive
> .inst (0xd5200000|((((2) << 19) | ((1) << 16) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 7) & 0x7)) << 12) | ((((((((((0x160 + (i * 4))))) >> 2))) & 0xf)) << 8) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 4) & 0x7)) << 5)))|(.L__reg_num_x8))
>       ^
> drivers/hwtracing/coresight/coresight-etm4x-core.c:702:4: note: while in macro instantiation
>                         etm4x_relaxed_read32(csa, TRCCNTVRn(i));
>                         ^
> drivers/hwtracing/coresight/coresight-etm4x.h:403:4: note: expanded from macro 'etm4x_relaxed_read32'
>                  read_etm4x_sysreg_offset((offset), false)))
>                  ^
> drivers/hwtracing/coresight/coresight-etm4x.h:383:12: note: expanded from macro 'read_etm4x_sysreg_offset'
>                         __val = read_etm4x_sysreg_const_offset((offset));       \
>                                 ^
> drivers/hwtracing/coresight/coresight-etm4x.h:149:2: note: expanded from macro 'read_etm4x_sysreg_const_offset'
>         READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))
>         ^
> drivers/hwtracing/coresight/coresight-etm4x.h:144:2: note: expanded from macro 'READ_ETM4x_REG'
>         read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG((reg)))
>         ^
> arch/arm64/include/asm/sysreg.h:1108:15: note: expanded from macro 'read_sysreg_s'
>         asm volatile(__mrs_s("%0", r) : "=r" (__val));                  \
>                      ^
> arch/arm64/include/asm/sysreg.h:1074:2: note: expanded from macro '__mrs_s'
> "       mrs_s " v ", " __stringify(r) "\n"                      \
>  ^
> 
> It appears that the __builin_constant_p() check in
> read_etm4x_sysreg_offset() falsely returns 'true' here because clang
> decides finds that an out-of-bounds access to config->cntr_val[] cannot
> happen, and then it unrolls the loop with constant register numbers. Then
> when actually emitting the output, it fails to figure out the value again.
> 
> While this is incorrect behavior in clang, it is easy to work around
> by avoiding the out-of-bounds array access. Do this by limiting the
> loop counter to the actual dimension of the array.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1310
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 15016f757828..4cccf874a602 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info)
>  			"timeout while waiting for PM stable Trace Status\n");
>  
>  	/* read the status of the single shot comparators */
> -	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> +	for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) {
>  		config->ss_status[i] =
>  			etm4x_relaxed_read32(csa, TRCSSCSRn(i));
>  	}
>  
>  	/* read back the current counter values */
> -	for (i = 0; i < drvdata->nr_cntr; i++) {
> +	for (i = 0; i < min_t(u32, drvdata->nr_cntr, ETMv4_MAX_CNTR); i++) {

This patch will work and I'd be happy to apply it if this was the only instance,
but there are dozens of places in the coresight drivers where such patterns are
used.  Why are those not flagged as well?  And shouldn't the real fix be with
clang?

Thanks,
Mathieu 

>  		config->cntr_val[i] =
>  			etm4x_relaxed_read32(csa, TRCCNTVRn(i));
>  	}
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] coresight: etm4x: work around clang-12+ build failure
  2021-02-25  9:42 [PATCH] coresight: etm4x: work around clang-12+ build failure Arnd Bergmann
  2021-02-25 16:45 ` Mathieu Poirier
@ 2021-02-25 17:20 ` Suzuki K Poulose
  1 sibling, 0 replies; 6+ messages in thread
From: Suzuki K Poulose @ 2021-02-25 17:20 UTC (permalink / raw)
  To: Arnd Bergmann, Mathieu Poirier, Alexander Shishkin,
	Nathan Chancellor, Nick Desaulniers
  Cc: Arnd Bergmann, Mike Leach, Leo Yan, Greg Kroah-Hartman,
	Sai Prakash Ranjan, coresight, linux-arm-kernel, linux-kernel,
	clang-built-linux

On 2/25/21 9:42 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang-12 fails to build the etm4x driver with -fsanitize=array-bounds:
> 
> <instantiation>:1:7: error: expected constant expression in '.inst' directive
> .inst (0xd5200000|((((2) << 19) | ((1) << 16) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 7) & 0x7)) << 12) | ((((((((((0x160 + (i * 4))))) >> 2))) & 0xf)) << 8) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 4) & 0x7)) << 5)))|(.L__reg_num_x8))
>        ^
> drivers/hwtracing/coresight/coresight-etm4x-core.c:702:4: note: while in macro instantiation
>                          etm4x_relaxed_read32(csa, TRCCNTVRn(i));
>                          ^
> drivers/hwtracing/coresight/coresight-etm4x.h:403:4: note: expanded from macro 'etm4x_relaxed_read32'
>                   read_etm4x_sysreg_offset((offset), false)))
>                   ^
> drivers/hwtracing/coresight/coresight-etm4x.h:383:12: note: expanded from macro 'read_etm4x_sysreg_offset'
>                          __val = read_etm4x_sysreg_const_offset((offset));       \
>                                  ^
> drivers/hwtracing/coresight/coresight-etm4x.h:149:2: note: expanded from macro 'read_etm4x_sysreg_const_offset'
>          READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))
>          ^
> drivers/hwtracing/coresight/coresight-etm4x.h:144:2: note: expanded from macro 'READ_ETM4x_REG'
>          read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG((reg)))
>          ^
> arch/arm64/include/asm/sysreg.h:1108:15: note: expanded from macro 'read_sysreg_s'
>          asm volatile(__mrs_s("%0", r) : "=r" (__val));                  \
>                       ^
> arch/arm64/include/asm/sysreg.h:1074:2: note: expanded from macro '__mrs_s'
> "       mrs_s " v ", " __stringify(r) "\n"                      \
>   ^
> 
> It appears that the __builin_constant_p() check in
> read_etm4x_sysreg_offset() falsely returns 'true' here because clang
> decides finds that an out-of-bounds access to config->cntr_val[] cannot

s/decides finds/decides/ ?

> happen, and then it unrolls the loop with constant register numbers. Then
> when actually emitting the output, it fails to figure out the value again.
> 
> While this is incorrect behavior in clang, it is easy to work around
> by avoiding the out-of-bounds array access. Do this by limiting the
> loop counter to the actual dimension of the array.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1310
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 15016f757828..4cccf874a602 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info)
>   			"timeout while waiting for PM stable Trace Status\n");
>   
>   	/* read the status of the single shot comparators */
> -	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> +	for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) {
>   		config->ss_status[i] =
>   			etm4x_relaxed_read32(csa, TRCSSCSRn(i));
>   	}
>   

There are more places where we do this. So I believe you need the similar
change elsewhere too in the driver. So, that becomes cumbersome for circumventing
the compiler issue.

Suzuki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] coresight: etm4x: work around clang-12+ build failure
  2021-02-25 16:45 ` Mathieu Poirier
@ 2021-02-25 20:08   ` Arnd Bergmann
  2021-02-25 21:23   ` Nick Desaulniers
  1 sibling, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2021-02-25 20:08 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Alexander Shishkin, Nathan Chancellor,
	Nick Desaulniers, Arnd Bergmann, Mike Leach, Leo Yan,
	Greg Kroah-Hartman, Sai Prakash Ranjan, coresight, Linux ARM,
	linux-kernel, clang-built-linux

On Thu, Feb 25, 2021 at 5:45 PM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On Thu, Feb 25, 2021 at 10:42:58AM +0100, Arnd Bergmann wrote:
> > index 15016f757828..4cccf874a602 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info)
> >                       "timeout while waiting for PM stable Trace Status\n");
> >
> >       /* read the status of the single shot comparators */
> > -     for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> > +     for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) {
> >               config->ss_status[i] =
> >                       etm4x_relaxed_read32(csa, TRCSSCSRn(i));
> >       }
> >
> >       /* read back the current counter values */
> > -     for (i = 0; i < drvdata->nr_cntr; i++) {
> > +     for (i = 0; i < min_t(u32, drvdata->nr_cntr, ETMv4_MAX_CNTR); i++) {
>
> This patch will work and I'd be happy to apply it if this was the only instance,
> but there are dozens of places in the coresight drivers where such patterns are
> used.  Why are those not flagged as well?  And shouldn't the real fix be with
> clang?

This is the only one I see in randconfig build tests. In fact I only actually
see the second one of the two causing problems, but it seemed silly
to change only one of them. I had not noticed the other similar
loops that use etm4x_read32() instead of etm4x_relaxed_read32().

I first suspected that the memory clobber in  __iormb() prevents the
compiler from running into the same problem there, but changing all
etm4x_read32() to etm4x_relaxed_read32() did not make it show up
in other places either.

While I expect this will be fixed in mainline clang, the workaround should
be harmless and let us use the existing releases as well.

      Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] coresight: etm4x: work around clang-12+ build failure
  2021-02-25 16:45 ` Mathieu Poirier
  2021-02-25 20:08   ` Arnd Bergmann
@ 2021-02-25 21:23   ` Nick Desaulniers
  2021-02-25 22:04     ` Arnd Bergmann
  1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2021-02-25 21:23 UTC (permalink / raw)
  To: Mathieu Poirier, Arnd Bergmann
  Cc: Suzuki K Poulose, Alexander Shishkin, Nathan Chancellor,
	Arnd Bergmann, Mike Leach, Leo Yan, Greg Kroah-Hartman,
	Sai Prakash Ranjan, coresight, Linux ARM, LKML,
	clang-built-linux, Bill Wendling

On Thu, Feb 25, 2021 at 8:45 AM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> Good morning,
>
> On Thu, Feb 25, 2021 at 10:42:58AM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > clang-12 fails to build the etm4x driver with -fsanitize=array-bounds:
> >
> > <instantiation>:1:7: error: expected constant expression in '.inst' directive
> > .inst (0xd5200000|((((2) << 19) | ((1) << 16) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 7) & 0x7)) << 12) | ((((((((((0x160 + (i * 4))))) >> 2))) & 0xf)) << 8) | (((((((((((0x160 + (i * 4))))) >> 2))) >> 4) & 0x7)) << 5)))|(.L__reg_num_x8))
> >       ^
> > drivers/hwtracing/coresight/coresight-etm4x-core.c:702:4: note: while in macro instantiation
> >                         etm4x_relaxed_read32(csa, TRCCNTVRn(i));
> >                         ^
> > drivers/hwtracing/coresight/coresight-etm4x.h:403:4: note: expanded from macro 'etm4x_relaxed_read32'
> >                  read_etm4x_sysreg_offset((offset), false)))
> >                  ^
> > drivers/hwtracing/coresight/coresight-etm4x.h:383:12: note: expanded from macro 'read_etm4x_sysreg_offset'
> >                         __val = read_etm4x_sysreg_const_offset((offset));       \
> >                                 ^
> > drivers/hwtracing/coresight/coresight-etm4x.h:149:2: note: expanded from macro 'read_etm4x_sysreg_const_offset'
> >         READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))
> >         ^
> > drivers/hwtracing/coresight/coresight-etm4x.h:144:2: note: expanded from macro 'READ_ETM4x_REG'
> >         read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG((reg)))
> >         ^
> > arch/arm64/include/asm/sysreg.h:1108:15: note: expanded from macro 'read_sysreg_s'
> >         asm volatile(__mrs_s("%0", r) : "=r" (__val));                  \
> >                      ^
> > arch/arm64/include/asm/sysreg.h:1074:2: note: expanded from macro '__mrs_s'
> > "       mrs_s " v ", " __stringify(r) "\n"                      \
> >  ^
> >
> > It appears that the __builin_constant_p() check in
> > read_etm4x_sysreg_offset() falsely returns 'true' here because clang
> > decides finds that an out-of-bounds access to config->cntr_val[] cannot
> > happen, and then it unrolls the loop with constant register numbers. Then

Is a sanitizer enabled, that would trap on OOB?

> > when actually emitting the output, it fails to figure out the value again.
> >
> > While this is incorrect behavior in clang, it is easy to work around
> > by avoiding the out-of-bounds array access. Do this by limiting the
> > loop counter to the actual dimension of the array.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1310
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index 15016f757828..4cccf874a602 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -691,13 +691,13 @@ static void etm4_disable_hw(void *info)
> >                       "timeout while waiting for PM stable Trace Status\n");
> >
> >       /* read the status of the single shot comparators */
> > -     for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> > +     for (i = 0; i < min_t(u32, drvdata->nr_ss_cmp, ETM_MAX_SS_CMP); i++) {
> >               config->ss_status[i] =
> >                       etm4x_relaxed_read32(csa, TRCSSCSRn(i));
> >       }
> >
> >       /* read back the current counter values */
> > -     for (i = 0; i < drvdata->nr_cntr; i++) {
> > +     for (i = 0; i < min_t(u32, drvdata->nr_cntr, ETMv4_MAX_CNTR); i++) {
>
> This patch will work and I'd be happy to apply it if this was the only instance,
> but there are dozens of places in the coresight drivers where such patterns are
> used.  Why are those not flagged as well?  And shouldn't the real fix be with
> clang?

It's important to understand the __builtin_constant_p is highly
sensitive to optimizations; code using it typically relies on
optimizations being performed before it's evaluated.  Which
optimizations, applied successfully or not, in what order, by which
compiler or versions of the same compiler can affect what
__builtin_constant_p evaluates to.  Code generally needs to be written
to assume that failure for __builtin_constant_p to evaluate to a
specific value or not is _not a bug_.

>
> Thanks,
> Mathieu
>
> >               config->cntr_val[i] =
> >                       etm4x_relaxed_read32(csa, TRCCNTVRn(i));
> >       }
> > --
> > 2.29.2
> >



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] coresight: etm4x: work around clang-12+ build failure
  2021-02-25 21:23   ` Nick Desaulniers
@ 2021-02-25 22:04     ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2021-02-25 22:04 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Mathieu Poirier, Suzuki K Poulose, Alexander Shishkin,
	Nathan Chancellor, Arnd Bergmann, Mike Leach, Leo Yan,
	Greg Kroah-Hartman, Sai Prakash Ranjan, coresight, Linux ARM,
	LKML, clang-built-linux, Bill Wendling

On Thu, Feb 25, 2021 at 10:23 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Feb 25, 2021 at 8:45 AM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > Good morning,
> >
> > On Thu, Feb 25, 2021 at 10:42:58AM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > >
> > > clang-12 fails to build the etm4x driver with -fsanitize=array-bounds:
>
> Is a sanitizer enabled, that would trap on OOB?
>
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1310

As described over there, this happens only with the array-bounds
sanitizer.

Actually, looking at it again now, in the reduced test case, the inline
assembly is not even parsable, it only works because it never gets
emitted without  -fsanitize=array-bounds, and the alternative
code path is used in

#define read_etm4x_sysreg_offset(offset, _64bit)
         \
        ({
         \
                u64 __val;
         \

         \
                if (__builtin_constant_p((offset)))
         \
                        __val =
read_etm4x_sysreg_const_offset((offset));       \
                else
         \
                        __val = etm4x_sysreg_read((offset), true,
(_64bit));    \
                __val;
         \
         })

read_etm4x_sysreg_const_offset() eventually turns into something
like

asm("msr_s " __stringify(offset));

so the offset has to be something that can be parsed by the
assembler. __builtin_constant_p() checks that it is a constant
value at compile-time, and in this case it can be because there
is a small upper bound and clang just unrolls the loop.

I don't think there is an alternative to __builtin_constant_p()
that can be used to decide if the argument is something that
can be used as a constant expression in an inline assembly.

            Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-25 22:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25  9:42 [PATCH] coresight: etm4x: work around clang-12+ build failure Arnd Bergmann
2021-02-25 16:45 ` Mathieu Poirier
2021-02-25 20:08   ` Arnd Bergmann
2021-02-25 21:23   ` Nick Desaulniers
2021-02-25 22:04     ` Arnd Bergmann
2021-02-25 17:20 ` Suzuki K Poulose

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).