Re: [PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as empty

[Jinyang He]

On 2024-08-21 15:52, Xi Ruoyao wrote:

> On Wed, 2024-08-21 at 15:37 +0800, Huacai Chen wrote:
> > > I am not sure whether the GCC bug has been fixed, I can not find the
> > > fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
> > > and in the GCC repo. So I am not sure whether it is time and proper
> > > to remove this workaround in the common header totally, just remove
> > > it in the arch specified header when compiling kernel with a newer
> > > GCC version (for example GCC 12.1 or higher on LoongArch) at least.
> > What's your opinion? From my point of view, this GCC bug hasn't been
> > fixed. So there may still be potential problems.
> I'm pretty sure it isn't fixed.  Using the test case from the bug
> report:
>
> struct i2c_board_info {
>          char type[20];
>          char pad[100];
> };
>
> #ifdef NONORETURN
> void fortify_panic();
> #else
> void fortify_panic() __attribute__((noreturn));
> #endif
>
>
> int f(int a)
> {
>    if (a)
>      fortify_panic();
> }
>
>
> void i2c_new_device(struct i2c_board_info *);
> int em28xx_dvb_init(int model, int a, int b, int c, int d)
> {
>          switch (model) {
>          case 1:{
>                          struct i2c_board_info info = {};
>                          f(a);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 2:{
>                          struct i2c_board_info info = {};
>                          f(b);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 3:{
>                          struct i2c_board_info info = { };
>                          f(c);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 4:{
>                          struct i2c_board_info info = { };
>                          f(d);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          }
>          return 0;
> }
>
> $ cc -v
> Using built-in specs.
> COLLECT_GCC=cc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/loongarch64-unknown-linux-gnu/14.2.0/lto-wrapper
> Target: loongarch64-unknown-linux-gnu
> Configured with: ../configure --prefix=/usr LD=ld --enable-languages=c,c++ --enable-default-pie --enable-default-ssp --disable-multilib --with-build-config=bootstrap-lto --disable-fixincludes --with-system-zlib --enable-host-pie
> Thread model: posix
> Supported LTO compression algorithms: zlib zstd
> gcc version 14.2.0 (GCC)
> $ cc t.c -S -Wframe-larger-than=1 -DNONORETURN -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 144 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
> $ cc t.c -S -Wframe-larger-than=1 -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 512 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
>
> And I'm puzzled why "unreachable instruction" is not a problem on x86?
Hi, Ruoyao,

I looked the gcc:machine_kexec.c:rtl:jump2+:kexec_reboot, it seems
the gcc thought the block `asm volatile ("")` is same. So for -Os,
if 2+ these blocks appears, it create "b" to the same block.

I tried "-fno-crossjumping" to disable the jump2 pass, it works.
Could we change this block different to solve this? (e.g. by __COUNTER__).
But it seems break the original trick. :-(

Jinyang