This is an archive of the discontinued LLVM Phabricator instance.

Differential D97743

Define __GCC_HAVE_DWARF2_CFI_ASM if applicable
ClosedPublic

Authored by MaskRay on Mar 1 2021, 4:08 PM.

Details

Reviewers
dexonsmith
jansvoboda11
Commits
rG9d117e7b2a39: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable
rGc11ff4bbada3: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable
Summary

In -fno-exceptions -fno-asynchronous-unwind-tables -g0 mode,
GCC does not emit .cfi_* directives.

% diff <(gcc -fno-asynchronous-unwind-tables -dM -E a.c) <(gcc -dM -E a.c)
130a131
> #define __GCC_HAVE_DWARF2_CFI_ASM 1

This macro is useful because code can decide whether inline asm should include .cfi_* directives.
.cfi_* directives without .cfi_startproc can cause assembler errors.

Diff Detail

Unit TestsFailed

TimeTest
480 msx64 debian > Clang Tools.clang-tidy/checkers::cppcoreguidelines-macro-usage-command-line-macros.cpp
30 msx64 debian > Clang Tools.pp-trace::pp-trace-include.cpp
30 msx64 debian > Clang Tools.pp-trace::pp-trace-macro.cpp
20 msx64 debian > Clang-Unit.Frontend/_/FrontendTests::CommandLineTest.RoundTrip
110 msx64 debian > Clang.CodeGen::debug-info-macro.c
View Full Test Results (14 Failed)

Event Timeline

MaskRay requested review of this revision.Mar 1 2021, 4:08 PM
MaskRay created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 4:08 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay updated this revision to Diff 327320.Mar 1 2021, 4:37 PM
MaskRay retitled this revision from Define __GCC_HAVE_DWARF2_CFI_ASM to 1 on ELF/Mach-O if CC1 -munwind-tables is specified to Define __GCC_HAVE_DWARF2_CFI_ASM if applicable.
MaskRay edited the summary of this revision. (Show Details)

Check -g and exceptions

Harbormaster completed remote builds in B91467: Diff 327311.Mar 1 2021, 9:20 PM
Harbormaster completed remote builds in B91473: Diff 327320.Mar 1 2021, 10:23 PM
MaskRay added a comment.Mar 4 2021, 11:51 AM

@jansvoboda11 :)

jansvoboda11 accepted this revision.Mar 9 2021, 4:16 AM

Could the logic be implemented in the driver?

LGTM regardless.

This revision is now accepted and ready to land.Mar 9 2021, 4:16 AM
MaskRay added a comment.EditedMar 9 2021, 10:32 AM
In D97743#2613629, @jansvoboda11 wrote:

Could the logic be implemented in the driver?

LGTM regardless.

No. Res is a clang::CompilerInvocation object. CompilerInvocation is not in the driver... -D -U and __CET__ are implemented in this file, too.

This revision was landed with ongoing or failed builds.Mar 9 2021, 10:52 AM
Closed by commit rGc11ff4bbada3: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable (authored by MaskRay). · Explain Why
This revision was automatically updated to reflect the committed changes.
MaskRay added a commit: rGc11ff4bbada3: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable.
thakis added a subscriber: thakis.Mar 9 2021, 11:56 AM

This seems to break tests everywhere: http://45.33.8.238/linux/41275/step_7.txt

MaskRay added a comment.Mar 9 2021, 12:05 PM
In D97743#2614792, @thakis wrote:

This seems to break tests everywhere: http://45.33.8.238/linux/41275/step_7.txt

My bad. Fixed by df67d3526962ae51446b1390e7c40e045e580ec2

thakis added a comment.Mar 9 2021, 12:11 PM

Still broken: http://45.33.8.238/linux/41278/step_7.txt

MaskRay added a reverting change: rGb4948c27d2e4: Revert D97743 "Define __GCC_HAVE_DWARF2_CFI_ASM if applicable".Mar 9 2021, 12:14 PM
MaskRay added a comment.Mar 9 2021, 12:15 PM
In D97743#2614827, @thakis wrote:

Still broken: http://45.33.8.238/linux/41278/step_7.txt

b4948c27d2e40586c0c4fa7cbf20b9c40a9f7584

The green Harbomaster misled me..

jansvoboda11 added a comment.Mar 9 2021, 12:34 PM
In D97743#2614480, @MaskRay wrote:
In D97743#2613629, @jansvoboda11 wrote:

Could the logic be implemented in the driver?

LGTM regardless.

No. Res is a clang::CompilerInvocation object. CompilerInvocation is not in the driver... -D -U and __CET__ are implemented in this file, too.

Right. I meant if it would be possible to move the logic to the driver and pass -D__GCC_HAVE_DWARF2_CFI_ASM=1 explicitly from the driver to -cc1, keeping CompilerInvocation as simple as possible.

MaskRay reopened this revision.Mar 9 2021, 12:38 PM
This revision is now accepted and ready to land.Mar 9 2021, 12:38 PM
MaskRay updated this revision to Diff 329441.Mar 9 2021, 12:38 PM

Move to driver

This revision was landed with ongoing or failed builds.Mar 9 2021, 10:21 PM
Closed by commit rG9d117e7b2a39: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable (authored by MaskRay). · Explain Why
This revision was automatically updated to reflect the committed changes.
MaskRay added a commit: rG9d117e7b2a39: Define __GCC_HAVE_DWARF2_CFI_ASM if applicable.
jansvoboda11 added a comment.Mar 9 2021, 11:48 PM
In D97743#2614899, @MaskRay wrote:

Move to driver

Thanks!

MaskRay added a comment.Mar 10 2021, 12:31 AM

There are some side effects: because -D is passed as command line options, it has <command line> in its filename, so isWrittenInBuiltinFile will not match it (isWrittenInBuiltinFile can match other built-in macros)...

Harbormaster completed remote builds in B92939: Diff 329441.Mar 10 2021, 1:23 AM

Revision Contents

PathSize
clang/
lib/
Frontend/
5 lines
test/
Preprocessor/
19 lines

Diff 327320

clang/lib/Frontend/CompilerInvocation.cpp

Show First 20 Lines • Show All 4,344 Lines • ▼ Show 20 Lines if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
Res.getCodeGenOpts().FineGrainedBitfieldAccesses = false; Res.getCodeGenOpts().FineGrainedBitfieldAccesses = false;
Diags.Report(diag::warn_drv_fine_grained_bitfield_accesses_ignored); Diags.Report(diag::warn_drv_fine_grained_bitfield_accesses_ignored);
} }
// Store the command-line for using in the CodeView backend. // Store the command-line for using in the CodeView backend.
Res.getCodeGenOpts().Argv0 = Argv0; Res.getCodeGenOpts().Argv0 = Argv0;
Res.getCodeGenOpts().CommandLineArgs = CommandLineArgs; Res.getCodeGenOpts().CommandLineArgs = CommandLineArgs;
if ((T.isOSBinFormatELF() || T.isOSBinFormatMachO()) &&
(Res.getLangOpts()->Exceptions || Res.getCodeGenOpts().UnwindTables ||
Res.getCodeGenOpts().getDebugInfo() != codegenoptions::NoDebugInfo))
Res.getPreprocessorOpts().addMacroDef("__GCC_HAVE_DWARF2_CFI_ASM=1");
Success &= FixupInvocation(Res, Diags, Args, DashX); Success &= FixupInvocation(Res, Diags, Args, DashX);
return Success; return Success;
} }
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Invocation, bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Invocation,
ArrayRef<const char *> CommandLineArgs, ArrayRef<const char *> CommandLineArgs,
DiagnosticsEngine &Diags, DiagnosticsEngine &Diags,
▲ Show 20 Lines • Show All 200 Lines • Show Last 20 Lines

clang/test/Preprocessor/unwind-tables.c

  • This file was added.
// RUN: %clang_cc1 %s -verify -munwind-tables -triple x86_64-windows
// RUN: %clang_cc1 %s -verify -triple x86_64-unknown-elf
// RUN: %clang_cc1 %s -verify -munwind-tables -DCFI_ASM -triple x86_64-unknown-elf
// RUN: %clang_cc1 %s -verify -munwind-tables -DCFI_ASM -triple aarch64-apple-darwin
// RUN: %clang_cc1 %s -verify -debug-info-kind=line-tables-only -DCFI_ASM -triple x86_64-unknown-elf
// RUN: %clang_cc1 %s -verify -fexceptions -DCFI_ASM -triple x86_64-unknown-elf
// expected-no-diagnostics
#ifdef CFI_ASM
#if __GCC_HAVE_DWARF2_CFI_ASM != 1
#error "__GCC_HAVE_DWARF2_CFI_ASM not defined"
#endif
#else
#ifdef __GCC_HAVE_DWARF2_CFI_ASM
#error "__GCC_HAVE_DWARF2_CFI_ASM defined"
#endif
#endif