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 40547 - Clang fails to pass validation when compiled with gcc-9 in release build.
Summary: Clang fails to pass validation when compiled with gcc-9 in release build.
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: -New Bugs (show other bugs)
Version: 8.0
Hardware: PC Linux
: P release blocker
Assignee: Unassigned Clang Bugs
URL:
Keywords:
: 42927 (view as bug list)
Depends on:
Blocks: release-9.0.1
  Show dependency tree
 
Reported: 2019-01-31 08:31 PST by sguelton
Modified: 2019-11-27 11:59 PST (History)
15 users (show)

See Also:
Fixed By Commit(s): r372281 7fc9f12


Attachments
adjusted patch to apply against latest trunk (1.24 KB, patch)
2019-08-15 15:06 PDT, Lone_Wolf
Details
Proposed patch (1.36 KB, patch)
2019-09-17 12:27 PDT, Martin Liška
Details

Note You need to log in before you can comment on or make changes to this bug.
Description sguelton 2019-01-31 08:31:36 PST
Building clang 8.0.0rc1 for fedora using gcc 9 compiles file, but validation fails in ~150 places.

When building with -O0, everything validates fine.
When building with -O1, everything validates fine.
When building with -O2, validation fails.
When building with -O2 -fsanitize=address,  everything validates fine.

Running clang under valgrind gives good hint on the error location:

valgrind /builddir/build/BUILD/cfe-8.0.0rc1.src/_build/bin/clang -cc1 -internal-isystem /builddir/build/BUILD/cfe-8.0.0rc1.src/_build/lib64/clang/8.0.0/include -nostdsysteminc -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm /builddir/build/BUILD/cfe-8.0.0rc1.src/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp -o -


==2582== Conditional jump or move depends on uninitialised value(s)
==2582==    at 0x880DC79: clang::CodeGen::CodeGenModule::ConstructAttributeList(llvm::StringRef, clang::CodeGen::CGFunctionInfo const&, clang::CodeGen::CGCalleeInfo, llvm::AttributeList&, unsigned int&, bool) (CGCall.cpp:2060)
==2582==    by 0x8A1E749: clang::CodeGen::CodeGenModule::SetLLVMFunctionAttributes(clang::GlobalDecl, clang::CodeGen::CGFunctionInfo const&, llvm::Function*) (CodeGenModule.cpp:1171)
==2582==    by 0x8A238AA: clang::CodeGen::CodeGenModule::SetFunctionAttributes(clang::GlobalDecl, llvm::Function*, bool, bool) (CodeGenModule.cpp:1548)
==2582==    by 0x8A5CA16: clang::CodeGen::CodeGenModule::GetOrCreateLLVMFunction(llvm::StringRef, llvm::Type*, clang::GlobalDecl, bool, bool, bool, llvm::AttributeList, clang::CodeGen::ForDefinition_t) (CodeGenModule.cpp:2824)
==2582==    by 0x87F7FAC: clang::CodeGen::CodeGenModule::getAddrOfCXXStructor(clang::CXXMethodDecl const*, clang::CodeGen::StructorType, clang::CodeGen::CGFunctionInfo const*, llvm::FunctionType*, bool, clang::CodeGen::ForDefinition_t) (CGCXX.cpp:253)
==2582==    by 0x882255E: clang::CodeGen::CodeGenFunction::EmitCXXConstructorCall(clang::CXXConstructorDecl const*, clang::CXXCtorType, bool, bool, clang::CodeGen::Address, clang::CodeGen::CallArgList&, clang::CodeGen::AggValueSlot::Overlap_t, clang::SourceLocation, bool) (CGClass.cpp:2134)
==2582==    by 0x882343C: clang::CodeGen::CodeGenFunction::EmitCXXConstructorCall(clang::CXXConstructorDecl const*, clang::CXXCtorType, bool, bool, clang::CodeGen::Address, clang::CXXConstructExpr const*, clang::CodeGen::AggValueSlot::Overlap_t, bool) (CGClass.cpp:2052)
==2582==    by 0x88B1B79: clang::CodeGen::CodeGenFunction::EmitCXXConstructEx

(...)
Comment 1 sguelton 2019-01-31 09:28:59 PST
The problem lies in the initialization of the bitfield members of ABIArgInfo.
Rewriting the default constructor to properly set each member fixes the error.
Comment 2 Paul Robinson 2019-01-31 09:34:51 PST
+hans for release bugs.
Comment 3 sguelton 2019-01-31 09:37:07 PST
Fix available in https://reviews.llvm.org/D57523
Comment 4 Hans Wennborg 2019-02-06 05:48:09 PST
(In reply to sguelton from comment #0)
> ==2582== Conditional jump or move depends on uninitialised value(s)
> ==2582==    at 0x880DC79:
> clang::CodeGen::CodeGenModule::ConstructAttributeList(llvm::StringRef,
> clang::CodeGen::CGFunctionInfo const&, clang::CodeGen::CGCalleeInfo,
> llvm::AttributeList&, unsigned int&, bool) (CGCall.cpp:2060)

CGCall.cpp:2060 at 8.0.0-rc1 is this line:

      else if (AI.getInReg())

that returns the InReg bitfield.

But that bitfield does get initialized (to false) in both of ABIArgInfo's constructors.

So I don't understand what the error is here exactly?
Comment 5 Hans Wennborg 2019-02-06 06:07:56 PST
Can you provide more information about exactly what gcc version you're using (I believe 9 doesn't even have a release branch yet), how your llvm build is configured, and in what way "validation" fails, i.e. an example of a test that's failing.
Comment 6 sguelton 2019-02-06 06:27:48 PST
Sure. 

GCC version is from the gcc-9 branch, that's the one used on fedora rawhide:

svn export svn://gcc.gnu.org/svn/gcc/branches/redhat/gcc-9-branch@268371

The llvm build uses RelWithDebInfo

You can get the whole setup here:

https://koji.fedoraproject.org/koji/taskinfo?taskID=32372028
Comment 7 sguelton 2019-02-06 06:30:55 PST
EDIT: the above link is for a rebuild of 7.01, but I had the same issue with 8.0.0rc1.
Comment 8 Hans Wennborg 2019-02-06 06:45:10 PST
(In reply to sguelton from comment #6)
> Sure. 
> 
> GCC version is from the gcc-9 branch, that's the one used on fedora rawhide:
> 
> svn export svn://gcc.gnu.org/svn/gcc/branches/redhat/gcc-9-branch@268371
> 
> The llvm build uses RelWithDebInfo
> 
> You can get the whole setup here:
> 
> https://koji.fedoraproject.org/koji/taskinfo?taskID=32372028

Perfect, I'll take a look.
Comment 9 sguelton 2019-02-07 04:56:54 PST
@hans: same problem when rebuilding from master with the following setup:

gcc built from svn export svn://gcc.gnu.org/svn/gcc/branches/redhat/gcc-9-branch@268371

llvm master

CXXFLAGS -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches  -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection

CFLAGS=CXXFLAGS

cmake options: ../llvm-project/llvm -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_CXX_COMPILER=/opt/notnfs/sergesanspaille/install/bin/g++ -DCMAKE_C_COMPILER=/opt/notnfs/sergesanspaille/install/bin/gcc
Comment 10 Hans Wennborg 2019-02-07 07:27:32 PST
I was able to reproduce this with the specified gcc version, except I only got four test failures:

Failing Tests (4):                                                                                                                                                                                
    Clang :: CodeGen/ppc64-extend.c                                                                                                                                                               
    Clang :: CodeGen/ppc64-inline-asm.c                                                                                                                                                           
    Clang :: CodeGenCXX/mips-size_t-ptrdiff_t.cpp                                                                                                                                                 
    Clang :: Driver/le32-unknown-nacl.cpp


I'm still not sure what's going on though. Maybe the next step is to try bisecting gcc.
Comment 11 Hans Wennborg 2019-02-07 07:32:04 PST Comment hidden (obsolete)
Comment 12 Hans Wennborg 2019-02-07 09:07:15 PST
(In reply to Hans Wennborg from comment #11)
> > I'm still not sure what's going on though. Maybe the next step is to try
> > bisecting gcc.
> 
> Does the redhat/gcc-9-branch branch exist in gcc's git mirror?

Never mind, I can reproduce with trunk gcc.
Comment 13 Hans Wennborg 2019-02-08 00:47:46 PST
My bisection last night failed, and I'll probably not have time to try again today, so if someone else wants to give it a try, please do so.
Comment 14 Hans Wennborg 2019-02-19 08:45:59 PST
I think my previous bisection failed because the test failure isn't completely deterministic. The same test doesn't fail each time.

I tried bisecting again, building gcc, then clang, then running check-all at each step, and it points to this:

---
75a6f948423540b728611660963d1740440d87eb is the first bad commit
commit 75a6f948423540b728611660963d1740440d87eb
Author: hubicka <hubicka@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sat Jan 12 18:18:23 2019 +0000

            * params.def (inline-unit-growth): Set to 40.
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@267883 138bc75d-0d04-0410-961f-82ee72b054a4

:040000 040000 73c34e4b0fc97de49ad228c609d495461e496b90 5882541f2e6fbcd712fd641f535238b4856ff045 M      gcc
---

That doesn't seem very interesting. If it is indeed the culprit, I guess it only uncovered some already-existing problem :-/
Comment 15 Hans Wennborg 2019-02-20 03:20:31 PST
I can't really spend more time on this. Landing https://reviews.llvm.org/D57523 seems like the way to go.
Comment 16 sguelton 2019-02-20 21:12:24 PST
Patch merged, thanks Hans!
Comment 17 Hans Wennborg 2019-02-21 01:07:12 PST
Looks like it was reverted in r354549. Let's keep this open until it sticks and also makes it to the release_80 branch.
Comment 18 Hans Wennborg 2019-02-25 08:10:23 PST
Serge, do you have any ideas for how to do this without breaking various GCC versions? :-)
Comment 19 sguelton 2019-02-25 08:20:48 PST
Not quite :-) As gcc 9 is not officially out yet, as of https://gcc.gnu.org/releases.html at least, I guess it's okay to still do the release, and eventually backport a patch if needed?
Comment 20 Hans Wennborg 2019-02-26 03:12:46 PST
(In reply to sguelton from comment #19)
> Not quite :-) As gcc 9 is not officially out yet, as of
> https://gcc.gnu.org/releases.html at least, I guess it's okay to still do
> the release, and eventually backport a patch if needed?

As we have an idea of how to work around it, it would still be nice to fix before release if possible.

Of course it would be even better if GCC was fixed before released. Do you know if there's a bug tracking this on the GCC side?
Comment 21 Hans Wennborg 2019-03-05 02:00:45 PST
> Of course it would be even better if GCC was fixed before released. Do you know if there's a bug tracking this on the GCC side?

Ping?
Comment 22 sguelton 2019-03-07 05:53:53 PST
I'll give it another try today :-)
Comment 23 Hans Wennborg 2019-03-08 01:31:03 PST
Unblocking the release from this.
Comment 24 Luke 2019-08-14 19:13:20 PDT
On Ubuntu 18.08, I'm seeing the following Failing Tests (5):
    Clang :: CodeGen/arm-fp16-arguments.c
    Clang :: CodeGen/lanai-regparm.c
caused by https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267883
	* params.def (inline-unit-growth): Set to 40.

    Clang :: CodeGen/ppc64-complex-parms.c
    Clang :: CodeGen/ppc64-complex-return.c
    Clang :: CodeGen/ppc64-vector.c
caused by https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268448
	* parms.def (MAX_INLINE_INSNS_SINGLE): Reduce from 400 to 200.


But these changes only caused failures in clang 7+. Bisecting clan on
    Clang :: CodeGen/ppc64-complex-parms.c
    Clang :: CodeGen/ppc64-complex-return.c
    Clang :: CodeGen/ppc64-vector.c
I get: 
http://llvm.org/viewvc/llvm-project?view=revision&revision=322396
	Refactor handling of signext/zeroext in ABIArgInfo

Alex Bradbury,
Could you take a look at this to see why gcc is over optimizing your code after r268448?
Comment 25 Tom Stellard 2019-08-14 19:58:26 PDT
*** Bug 42927 has been marked as a duplicate of this bug. ***
Comment 26 Tom Stellard 2019-08-14 20:01:02 PDT
I'm still getting around 129 test failures when building clang with gcc-9 on Fedora 31.
Comment 27 Tom Stellard 2019-08-14 20:02:01 PDT
Also, re-applying https://reviews.llvm.org/rL354546 fixes the failures for me.
Comment 28 Jan Hubicka 2019-08-15 03:57:36 PDT
The patch changing inliner metrics should not trigger wrong code, so indeed it probably just uncovers latent problem. Is it understood whether this is a GCC bug and if so, do you have a testcase?

Note that I have also run into misoptimization issues while compiling clang with GCC 9 and FDO. I had no chance to debug it, but it was crashing after parsing even quite simple source file.  It would be great to figure out where the problem is.

To compile with profile feedback you need to pass -fprofile-generate in addition to standard flags and build whole tree, then train by testsuite and then rebuilding with -fprofile-use.
Comment 29 Lone_Wolf 2019-08-15 15:04:21 PDT
(In reply to Tom Stellard from comment #27)
> Also, re-applying https://reviews.llvm.org/rL354546 fixes the failures for
> me.

I had trouble re-applying that cleanly due to the changes in https://reviews.llvm.org/D60349 and created the attached patch.

I'm not a programmer or coder, please verify the attached patch.
Comment 30 Lone_Wolf 2019-08-15 15:06:38 PDT
Created attachment 22381 [details]
adjusted patch to apply against latest trunk
Comment 31 Luke 2019-08-16 08:02:57 PDT
One more data point. I bisected Clang against:
    Clang :: CodeGen/arm-fp16-arguments.c
    Clang :: CodeGen/lanai-regparm.c

which pointed to:
http://llvm.org/viewvc/llvm-project?view=revision&revision=337514
    Reapply "ADT: Shrink size of SmallVector by 8B on 64-bit platforms"

Duncan,
This commit appears to be be causing gcc to over optimize.
Comment 32 Duncan 2019-08-16 08:17:50 PDT
(In reply to Luke from comment #31)
> One more data point. I bisected Clang against:
>     Clang :: CodeGen/arm-fp16-arguments.c
>     Clang :: CodeGen/lanai-regparm.c
> 
> which pointed to:
> http://llvm.org/viewvc/llvm-project?view=revision&revision=337514
>     Reapply "ADT: Shrink size of SmallVector by 8B on 64-bit platforms"
> 
> Duncan,
> This commit appears to be be causing gcc to over optimize.

Interesting!  It caused problems in very old (unsupported) versions of clang too, likely due to a bug in the AArch64 backend.  I won’t be able to investigate GCC to figure out the bug, but let me know if you want more details about what that commit did.  I suspect it’s a recent regression in GCC since that commit landed over a year ago in Clang.
Comment 33 Luke 2019-08-20 05:31:35 PDT Comment hidden (obsolete)
Comment 34 Hans Wennborg 2019-08-27 05:16:38 PDT
It would be nice to get to the bottom of this, but I don't think we can block the release on it.
Comment 35 Luke 2019-09-12 08:38:47 PDT
I made another attempt to bisect gcc. Rather target a specific failure, this time I bisected against any validation failure. This points to
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261089
     gimple-ssa-store-merging.c: Include gimple-fold.h.

While subsequent gcc tweaks resulted in different tests failing, this commit appears to be the genesis of all of the failures.
Comment 36 Tom Stellard 2019-09-12 09:09:37 PDT
(In reply to Luke from comment #35)
> I made another attempt to bisect gcc. Rather target a specific failure, this
> time I bisected against any validation failure. This points to
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261089
>      gimple-ssa-store-merging.c: Include gimple-fold.h.
> 
> While subsequent gcc tweaks resulted in different tests failing, this commit
> appears to be the genesis of all of the failures.

Have you filed a gcc bug for this?
Comment 37 Luke 2019-09-12 10:23:07 PDT
(In reply to Tom Stellard from comment #36)
> Have you filed a gcc bug for this?

Done. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

Please add any useful information that I may have left out, as I am neither a clang nor gcc developer.
Comment 38 Martin Liška 2019-09-17 10:18:21 PDT
Ok, I've got a patch. It's an issue in clang where ABIArgInfo constructor does not initialize some of the bit fields and then we end up with usage of an uninitialized variables. Following patch should fix it:

diff --git a/clang/include/clang/CodeGen/CGFunctionInfo.h b/clang/include/clang/CodeGen/CGFunctionInfo.h
index 1f81072e23d..ec2567555a3 100644
--- a/clang/include/clang/CodeGen/CGFunctionInfo.h
+++ b/clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -109,14 +109,11 @@ private:
     UnpaddedCoerceAndExpandType = T;
   }
 
-  ABIArgInfo(Kind K)
-      : TheKind(K), PaddingInReg(false), InReg(false) {
-  }
-
 public:
-  ABIArgInfo()
-      : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-        TheKind(Direct), PaddingInReg(false), InReg(false) {}
+  ABIArgInfo(Kind K = Direct)
+      : TheKind(K), PaddingInReg(false), InAllocaSRet(false),
+        IndirectByVal(false), IndirectRealign(false), SRetAfterThis(false),
+        InReg(false), CanBeFlattened(false), SignExt(false) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
                               llvm::Type *Padding = nullptr,

I'm going to send the patch to Phabricator.
Comment 39 Tom Stellard 2019-09-17 10:24:12 PDT
Have you tested this patch with gcc-7?  This similar patch was committed and reverted due to ICE in gcc-7: https://reviews.llvm.org/D57523
Comment 40 Martin Liška 2019-09-17 10:31:11 PDT
(In reply to Tom Stellard from comment #39)
> Have you tested this patch with gcc-7?  This similar patch was committed and
> reverted due to ICE in gcc-7: https://reviews.llvm.org/D57523

Ah, you had the patch. Yes, the patch should be re-applied!
I'll rebuild it with gcc-7 to see the ICE.
Do you have a link to the ICE please?
Comment 41 Tom Stellard 2019-09-17 10:33:37 PDT
(In reply to Martin Liška from comment #40)
> (In reply to Tom Stellard from comment #39)
> > Have you tested this patch with gcc-7?  This similar patch was committed and
> > reverted due to ICE in gcc-7: https://reviews.llvm.org/D57523
> 
> Ah, you had the patch. Yes, the patch should be re-applied!
> I'll rebuild it with gcc-7 to see the ICE.
> Do you have a link to the ICE please?

There was a link in the revert commit, but the logs have been deleted now:
https://reviews.llvm.org/rL354549
Comment 42 Martin Liška 2019-09-17 10:48:19 PDT
> There was a link in the revert commit, but the logs have been deleted now:
> https://reviews.llvm.org/rL354549

Ok, I'm rebuilding clang with the CC=gcc-7 CXX=g++-7 with the very same instructions like here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758#c0

The ICE was a normal ICE in GCC compiler during compilation of a clang object file, right?
Comment 43 Martin Liška 2019-09-17 11:38:37 PDT
(In reply to Martin Liška from comment #42)
> > There was a link in the revert commit, but the logs have been deleted now:
> > https://reviews.llvm.org/rL354549
> 
> Ok, I'm rebuilding clang with the CC=gcc-7 CXX=g++-7 with the very same
> instructions like here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758#c0
> 
> The ICE was a normal ICE in GCC compiler during compilation of a clang
> object file, right?

I can't see the ICE with:
g++-7 --version
g++-7 (SUSE Linux) 7.4.1 20190725 [gcc-7-branch revision 273795]

So please reapply the patch back.
Comment 44 Tom Stellard 2019-09-17 11:39:28 PDT
(In reply to Martin Liška from comment #43)
> (In reply to Martin Liška from comment #42)
> > > There was a link in the revert commit, but the logs have been deleted now:
> > > https://reviews.llvm.org/rL354549
> > 
> > Ok, I'm rebuilding clang with the CC=gcc-7 CXX=g++-7 with the very same
> > instructions like here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758#c0
> > 
> > The ICE was a normal ICE in GCC compiler during compilation of a clang
> > object file, right?
> 
> I can't see the ICE with:
> g++-7 --version
> g++-7 (SUSE Linux) 7.4.1 20190725 [gcc-7-branch revision 273795]
> 
> So please reapply the patch back.

Can you submit your patch?  The old patch doesn't apply cleanly any more.
Comment 45 Martin Liška 2019-09-17 12:27:44 PDT
Created attachment 22516 [details]
Proposed patch

Please make a patch submission on my behalf. Thanks.
Comment 46 Hans Wennborg 2019-09-18 00:23:46 PDT
(In reply to Martin Liška from comment #38)
> Ok, I've got a patch. It's an issue in clang where ABIArgInfo constructor
> does not initialize some of the bit fields and then we end up with usage of
> an uninitialized variables.

Can you point to where the uninitialized variable gets used? I stared hard at the code but couldn't find it.

Maybe initializing all of them is a good idea in any case though.
Comment 47 Martin Liška 2019-09-18 00:26:58 PDT
(In reply to Hans Wennborg from comment #46)
> (In reply to Martin Liška from comment #38)
> > Ok, I've got a patch. It's an issue in clang where ABIArgInfo constructor
> > does not initialize some of the bit fields and then we end up with usage of
> > an uninitialized variables.
> 
> Can you point to where the uninitialized variable gets used? I stared hard
> at the code but couldn't find it.

It happens in void computeInfo(CGFunctionInfo &FI) const override {:
      I.info = classifyArgumentType(I.type, State);

Here there's an assignment which eventually will copy members of the ABIArgInfo.

> 
> Maybe initializing all of them is a good idea in any case though.
Comment 48 sguelton 2019-09-18 17:59:27 PDT
Commited as https://reviews.llvm.org/rGe93aded7f02d661234ad81aac0785ccdef6a79dd
Let's hope there's no gcc regression this time :-/
Comment 49 Luke 2019-09-21 18:00:26 PDT
Verified that r372281 fixes the issue. Both 
gcc-7: gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
gcc-10: gcc-8.3 (GCC) 10.0.0 20190822 (experimental)

'make check-all' for clang r372468 with no failures.
Comment 50 Michał Górny 2019-11-22 23:58:29 PST
Can we get this backported to 9.0.1 (if it isn't yet)?
Comment 51 Tom Stellard 2019-11-27 11:59:56 PST
Merged: 7fc9f12