LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 42580 - missed optimization leading to compiletime assert with address sanitizer
Summary: missed optimization leading to compiletime assert with address sanitizer
Status: NEW
Alias: None
Product: clang
Classification: Unclassified
Component: -New Bugs (show other bugs)
Version: trunk
Hardware: PC Linux
: P enhancement
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks: 4068
  Show dependency tree
 
Reported: 2019-07-11 03:01 PDT by Arnd Bergmann
Modified: 2019-07-11 17:19 PDT (History)
8 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Bergmann 2019-07-11 03:01:33 PDT
The current clang-9 prerelease causes one file in the linux kernel to get fail compilation with an assertion triggering when building with -sanitize=kernel-address:

void __iwl_err(char *, ...);
int a;
void iwl_fw_dbg_apply_point() {
  const char b[] = "WRT ext=%d. Invalid apply point %d for %s\n";
  if (a) {
    void __compiletime_assert_2790(void);
    if (b[sizeof(b) - 2] != '\n')
      __compiletime_assert_2790();
  }
  __iwl_err(0, b);
}

$ clang-9 -O2 -fsanitize=kernel-address  -S dbg.i -o-    | grep compiletime
	callq	__compiletime_assert_2790

both clang-8 and gcc-8 pass the assertion here.

See also https://godbolt.org/z/cPrkv4
Comment 1 Nick Desaulniers 2019-07-11 11:01:44 PDT
See also: https://github.com/ClangBuiltLinux/linux/issues/580
Comment 2 Nathan Chancellor 2019-07-11 13:20:02 PDT
I bisected using Arnd's reproducer:

3bf72d7d64b8465acd4f4af1a469d68d9dc86058 is the first bad commit
commit 3bf72d7d64b8465acd4f4af1a469d68d9dc86058
Author: Eli Friedman <efriedma@quicinc.com>
Date:   Fri Feb 8 21:18:46 2019 +0000

    [Sema] Make string literal init an rvalue.
    
    This allows substantially simplifying the expression evaluation code,
    because we don't have to special-case lvalues which are actually string
    literal initialization.
    
    This currently throws away an optimization where we would avoid creating
    an array APValue for string literal initialization.  If we really want
    to optimize this case, we should fix APValue so it can store simple
    arrays more efficiently, like llvm::ConstantDataArray.  This shouldn't
    affect the memory usage for other string literals.  (Not sure if this is
    a blocker; I don't think string literal init is common enough for this
    to be a serious issue, but I could be wrong.)
    
    The change to test/CodeGenObjC/encode-test.m is a weird side-effect of
    these changes: we currently don't constant-evaluate arrays in C, so the
    strlen call shouldn't be folded, but lvalue string init managed to get
    around that check.  I this this is fine.
    
    Fixes https://bugs.llvm.org/show_bug.cgi?id=40430 .
    
    llvm-svn: 353569

 clang/lib/AST/ExprConstant.cpp                   | 69 +++++++++---------------
 clang/lib/CodeGen/CGExprConstant.cpp             | 10 ----
 clang/lib/Sema/SemaInit.cpp                      |  1 +
 clang/test/AST/ast-dump-wchar.cpp                |  8 +--
 clang/test/CodeGenObjC/encode-test.m             |  3 +-
 clang/test/SemaCXX/constant-expression-cxx11.cpp |  8 +++
 6 files changed, 40 insertions(+), 59 deletions(-)

$ git bisect log
git bisect start
# good: [7b5565418f4d6e113ba805dad40d471d23bca6f6] Fix build breakage from llvm r351317
git bisect good 7b5565418f4d6e113ba805dad40d471d23bca6f6
# bad: [617df204b5b466bd5688b74067e246dd7520ae65] [CodeGen] Add larger vector types for i32 and f32
git bisect bad 617df204b5b466bd5688b74067e246dd7520ae65
# bad: [5e165fba3acdb15bc7bb8ef8d44a2182ddda5e71] [NFC] Add missing revision number in libc++ ABI changelog
git bisect bad 5e165fba3acdb15bc7bb8ef8d44a2182ddda5e71
# bad: [7f78d4712f91d034e1850d0a0030a090fbe14999] [DebugInfo] Apply subprogram attributes on behalf of owner CU
git bisect bad 7f78d4712f91d034e1850d0a0030a090fbe14999
# good: [bd3adbb899f55247f6a162c3a1fde2f88b39a0df] [X86][SSE] Add tests showing missing SimplifyDemandedVectorElts support for X86ISD::BLENDV
git bisect good bd3adbb899f55247f6a162c3a1fde2f88b39a0df
# bad: [9e5e868d95c25c1c5b09f315cc9f7f15885d61a2] AMDGPU/GlobalISel: Fix RegBankSelect for GEP.
git bisect bad 9e5e868d95c25c1c5b09f315cc9f7f15885d61a2
# bad: [a2f60933e5c90bde4b383b2528ba1c142d920a01] llvm-lib: Implement /list flag
git bisect bad a2f60933e5c90bde4b383b2528ba1c142d920a01
# good: [baf2f35ec4c5adeaab2a51b2e6c5eb6156f82575] sanitizers: Introduce ThreadType enum
git bisect good baf2f35ec4c5adeaab2a51b2e6c5eb6156f82575
# good: [340cb87e833b0347401e44757d7fbcb88b677f5f] [llvm-objcopy] Add --redefine-syms
git bisect good 340cb87e833b0347401e44757d7fbcb88b677f5f
# good: [2add627e350161cd84e77fb4717483daf69782e5] [analyzer] Opt-in C Style Cast Checker for OSObject pointers
git bisect good 2add627e350161cd84e77fb4717483daf69782e5
# bad: [b041a18bcf98dcef8fc32332eb167ea510364921] Temporarily disable calls to getgrnam/getgrnam_r in test due to it hitting unrelated issues in EGLIBC 2.19.
git bisect bad b041a18bcf98dcef8fc32332eb167ea510364921
# bad: [1386d99c777f3ac2443f815df5f6ec353afe3e9b] [x86] add test for miscompiling setcc transform (PR40657); NFC
git bisect bad 1386d99c777f3ac2443f815df5f6ec353afe3e9b
# bad: [3edf63c55a1e9ae98afa6668f65c30fa8d6f353b] [lld-link] better error message when failing to open archive members
git bisect bad 3edf63c55a1e9ae98afa6668f65c30fa8d6f353b
# bad: [3bf72d7d64b8465acd4f4af1a469d68d9dc86058] [Sema] Make string literal init an rvalue.
git bisect bad 3bf72d7d64b8465acd4f4af1a469d68d9dc86058
# good: [57e60a501ed20f0e6483bad9535fac93312b8e77] Fix typo
git bisect good 57e60a501ed20f0e6483bad9535fac93312b8e77
# first bad commit: [3bf72d7d64b8465acd4f4af1a469d68d9dc86058] [Sema] Make string literal init an rvalue.
Comment 3 Eli Friedman 2019-07-11 16:36:47 PDT
It looks like without my patch, "b[sizeof(b) - 2]" got constant-folded by clang; with my patch, it doesn't get constant-folded by clang.  This is basically the same thing that happened to encode-test.m, and wasn't really intentional, as far as I can tell.

Without asan, the load is eventually optimized away by GVN.  With asan, we explicitly disable the optimization in question (see https://reviews.llvm.org/D14763).

I see a few options here to move forward:

1. Enable constant folding for all arrays in C, which would again catch this case.  This would probably require fixing the performance issue called out in the "FIXME" in r353569.
2. Some more restricted version of the above that specifically applies to string initialization, or a related hack.
3. Fix GVN so it's less conservative with asan enabled.
4. Ask the kernel team to fix their code, by adding "static" to the definition of the string. This is probably a good idea anyway.
Comment 4 Nick Desaulniers 2019-07-11 17:05:47 PDT
(In reply to Eli Friedman from comment #3)
> 4. Ask the kernel team to fix their code, by adding "static" to the
> definition of the string. This is probably a good idea anyway.

Thanks for the tip, will send such a patch (I've confirmed it works around the issue).
Comment 5 Nick Desaulniers 2019-07-11 17:19:44 PDT
https://lkml.org/lkml/2019/7/11/751