[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8257928: Test image build failure with clang-10 due to -Wmisleading-indentation
From:       Chris Plummer <cjplummer () openjdk ! java ! net>
Date:       2020-12-22 19:19:55
Message-ID: 7e-w6rdmO_JE0zmLCVAptzuLlL6nLZ55ULxpqJEYu9c=.700ab28a-7e4a-4093-9110-379159dd9112 () github ! com
[Download RAW message or body]

On Wed, 9 Dec 2020 07:11:53 GMT, Hao Sun <github.com+16932759+shqking@openjdk.org> \
wrote:

> Flag '-Wmisleading-indentation' was introduced since clang-10 [1] and
> gcc-6 [2]. Putting the code with proper indentations would suppress this
> warning.
> 
> The main reason why test image build with gcc succeeds is that, clang
> and gcc might behave differently for some corner cases under
> '-Wmisleading-indentation'.
> 
> The following code snippet is crafted based on this build failure, where
> each statement with improper indentation (line A and line B) follows an
> if-statement. The key difference between foo() and bar() lies in that
> there exists an extra comment, i.e. "/* comment 2 */", between statement
> at line A and the if-statement.
> 
> int foo(int num) {
> /* comment 1 */
> if (num > 1)
> return 0;
> 
> /* comment 2 */
> num = num + 1;  // line A
> return num;
> }
> 
> int bar(int val) {
> /* comment 3 */
> if (val > 1)
> return 0;
> 
> val = val + 1;  // line B
> return val;
> }
> 
> It's worth noting that foo() is quite similar to the erroneous code in
> this build failure. Clang-10 would emit misleading indentation warnings
> for both foo() and bar(), whereas gcc-6 or higher considers foo() free
> of problems. [3]
> 
> This patch is a complement to JDK-8253892. In JDK-8253892, flag
> '-Wmisleading-indentation' is disabled for both clang and gcc when
> building JRE and JDK images. This patch does not disable the flag for
> test image building, but just fixes the code, becasue:
> - only a small number of warnings are produced and it's easy to fix
> them one by one.
> - inconsistent warning behaviors between gcc and clang are triggered
> by this case and simply disabling the flag does not work.
> 
> Note that AArch64 is NOT tested because test image build still fails
> after this bug is fixed due to another runtime error (See JDK-8257936),
> which is not related to this patch.
> 
> [1] https://releases.llvm.org/10.0.0/tools/clang/docs/DiagnosticsReference.html
> [2] https://gcc.gnu.org/onlinedocs/gcc-6.5.0/gcc/Warning-Options.html#Warning-Options
>  [3] https://godbolt.org/z/xs6xWv
> 
> ---------------------
> We have tested with this patch, the test image can be built successfully with \
> clang-10 on Linux X86 machine.

Marked as reviewed by cjplummer (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/1709


[prev in list] [next in list] [prev in thread] [next in thread]