This is an archive of the discontinued LLVM Phabricator instance.

Differential D6366

Fix Itanium ABI for ARM
Needs ReviewPublic

Authored by weimingz on Nov 21 2014, 4:51 PM.
This revision needs review, but all specified reviewers are disabled or inactive.

Details

Reviewers
• espindola
Summary

r218141 "In the Itanium ABI, move stuff to the comdat of variables with static init." causes gnu as crash for ARM in some cases:

foo.s: Assembler messages:
foo.s:208: Internal error!
Assertion failure in get_line_subseg at crosstool_linaro/src/crosstool-ng-linaro-1.13.1-4.8-2014.01/builds/arm-linux-gnueabi-linux/.build/src/binutils-linaro-2.24-2013.12/gas/dwarf2dbg.c line 271.

r218141 handles the comdat of varialbes for Itanium ABI. However, the routine is shared by both ARM and Itanium.
It seems the change should be guarded to Itanium only.

Diff Detail

Event Timeline

weimingz updated this revision to Diff 16519.Nov 21 2014, 4:51 PM
weimingz retitled this revision from to Fix Itanium ABI for ARM.
weimingz updated this object.
weimingz edited the test plan for this revision. (Show Details)
weimingz added a reviewer: • rafael.
weimingz added a subscriber: Unknown Object (MLST).
Herald added a subscriber: aemerson. · View Herald TranscriptNov 21 2014, 4:51 PM
t.p.northover added a subscriber: t.p.northover.Nov 21 2014, 7:59 PM

Hi Weiming,

This doesn't look like it's actually implementing either the Itanium or ARM C++ ABI rules. Both specify that the guard variable should be in a COMDAT, they just disagree slightly about the details.

Cheers.

Tim.

weimingz added a comment.Nov 22 2014, 1:44 PM

Hi Tim,

I'm not sure if it's clang issue or GNU as issue.
I uploaded test cases in http://llvm.org/bugs/show_bug.cgi?id=21644
The generated asms w/ and w/o this patch are also attached.

Please help to take a look.

t.p.northover added a comment.Nov 22 2014, 3:24 PM

I see no evidence that this is an LLVM issue. Certainly all Itanium ELF ABIs require the guard variable to be in *some* comdat, and the ARM difference (section 3.2.6 of the C++ ABI) only applies to some fairly obscure linking situations[*] -- not during assembly.

Unfortunately I can't reproduce the issue with my version of as though (only 2.23) so I don't know what's going wrong there. Actually, I probably wouldn't anyway. If it's crashing GNU as, you need a GNU as developer to find out why.

Perhaps they'll come back with a real problem in what we emit, but I don't think it's this.

Cheers.

Tim.

  • Though we do seem to get it wrong for those situations: the guard variable apparently shouldn't be weak.
• espindola edited reviewers, added: • espindola; removed: • rafael.Mar 15 2018, 11:03 AM
Herald added subscribers: JDevlieghere, kristof.beyls, aprantl. · View Herald TranscriptMar 15 2018, 11:03 AM
aprantl removed a subscriber: aprantl.Mar 15 2018, 11:04 AM
JDevlieghere removed a subscriber: JDevlieghere.Mar 16 2018, 6:28 AM

Revision Contents

PathSize
lib/
CodeGen/
3 lines
test/
CodeGenCXX/
3 lines

Diff 16519

lib/CodeGen/ItaniumCXXABI.cpp

Context not available.
// The ABI says: It is suggested that it be emitted in the same COMDAT group // The ABI says: It is suggested that it be emitted in the same COMDAT group
// as the associated data object // as the associated data object
if (!D.isLocalVarDecl() && var->isWeakForLinker() && CGM.supportsCOMDAT()) { if (!UseARMGuardVarABI && !D.isLocalVarDecl() && var->isWeakForLinker()
&& CGM.supportsCOMDAT()) {
llvm::Comdat *C = CGM.getModule().getOrInsertComdat(var->getName()); llvm::Comdat *C = CGM.getModule().getOrInsertComdat(var->getName());
guard->setComdat(C); guard->setComdat(C);
var->setComdat(C); var->setComdat(C);
Context not available.

test/CodeGenCXX/static-data-member.cpp

// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s // RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | \ // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | \
// RUN: FileCheck --check-prefix=MACHO %s // RUN: FileCheck --check-prefix=MACHO %s
// RUN: %clang_cc1 -triple arm-apple-darwin -emit-llvm -o - %s | \
// RUN: FileCheck --check-prefix=ARM %s
// CHECK: @_ZN5test11A1aE = constant i32 10, align 4 // CHECK: @_ZN5test11A1aE = constant i32 10, align 4
// CHECK: @_ZN5test212_GLOBAL__N_11AIiE1xE = internal global i32 0, align 4 // CHECK: @_ZN5test212_GLOBAL__N_11AIiE1xE = internal global i32 0, align 4
Context not available.
// CHECK: @_ZGVN5test31AIiE1xE = weak_odr global i64 0, comdat $_ZN5test31AIiE1xE // CHECK: @_ZGVN5test31AIiE1xE = weak_odr global i64 0, comdat $_ZN5test31AIiE1xE
// MACHO: @_ZGVN5test31AIiE1xE = weak_odr global i64 0 // MACHO: @_ZGVN5test31AIiE1xE = weak_odr global i64 0
// MACHO-NOT: comdat // MACHO-NOT: comdat
// ARM-NOT: comdat
// CHECK: _ZN5test51U2k0E = global i32 0 // CHECK: _ZN5test51U2k0E = global i32 0
// CHECK: _ZN5test51U2k1E = global i32 0 // CHECK: _ZN5test51U2k1E = global i32 0
Context not available.