[llvm] r349976 - [MC] Enable .file support on COFF and diagnose it on unsupported targets

Thu Dec 27 17:22:55 PST 2018

This broke our macOS builders when building XRay with the following error:

FAILED:
compiler-rt/lib/xray/CMakeFiles/RTXray.osx.dir/xray_trampoline_x86_64.S.o
/b/s/w/ir/kitchen-workdir/recipe_cleanup/clangfJXnpj/llvm_build_dir/./bin/clang
--target=x86_64-apple-darwin17.7.0 -DXRAY_HAS_EXCEPTIONS=1 -D_DEBUG
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-I/b/s/w/ir/kitchen-workdir/llvm-project/compiler-rt/lib/xray/..
-I/b/s/w/ir/kitchen-workdir/llvm-project/compiler-rt/lib/xray/../../include
-fPIC -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra
-Wno-unused-parameter -Wwrite-strings -Wmissing-field-initializers
-Wimplicit-fallthrough -Wcovered-switch-default -Wdelete-non-virtual-dtor
-Wstring-conversion -fdiagnostics-color -Wall -Wno-unused-parameter -O3
-arch x86_64   -UNDEBUG  -stdlib=libc++ -mmacosx-version-min=10.9 -isysroot
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk
-fPIC -fno-builtin -fno-exceptions -funwind-tables -fno-stack-protector
-fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3
-gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions
-Wno-non-virtual-dtor -fno-rtti -MD -MT
compiler-rt/lib/xray/CMakeFiles/RTXray.osx.dir/xray_trampoline_x86_64.S.o
-MF
compiler-rt/lib/xray/CMakeFiles/RTXray.osx.dir/xray_trampoline_x86_64.S.o.d
-o
compiler-rt/lib/xray/CMakeFiles/RTXray.osx.dir/xray_trampoline_x86_64.S.o
 -c
/b/s/w/ir/kitchen-workdir/llvm-project/compiler-rt/lib/xray/xray_trampoline_x86_64.S
/b/s/w/ir/kitchen-workdir/llvm-project/compiler-rt/lib/xray/xray_trampoline_x86_64.S:97:2:
error: target does not support '.file' without a number
 .file "xray_trampoline_x86.S"
 ^

This has been compiling before so I don't understand why it hasn't crashed
the compiler like your file-single.s test does. In any case, we probably
need to change xray_trampoline_x86_64.S or revert this change.

On Fri, Dec 21, 2018 at 3:38 PM Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Fri Dec 21 15:35:48 2018
> New Revision: 349976
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349976&view=rev
> Log:
> [MC] Enable .file support on COFF and diagnose it on unsupported targets
>
> Summary:
> The "single parameter" .file directive appears to be an ELF-only feature
> that is intended to insert the main source filename into the string
> table table.
>
> I noticed that if you assemble an ELF .s file for COFF, typically it
> will assert right away on a .file directive near the top of the file. My
> first change was to make this emit a proper error in the asm parser so
> that we don't assert so easily.
>
> However, COFF actually does have some support for this directive, and if
> you emit an object file, llvm-mc does not assert. When emitting a COFF
> object, MC will take those file names and create "debug" symbol table
> entries for them. I'm not familiar with these kinds of symbol table
> entries, and I'm not aware of any users of them, but @compnerd added
> them a while ago. They don't introduce absolute paths, and most main
> source file paths are short enough that this extra entry shouldn't cause
> any problems, so I enabled the flag in MCAsmInfoCOFF that indicates that
> it's supported.
>
> This has the side effect of adding an extra debug symbol to every object
> produced by clang, which is a pretty big functional change. My question
> is, should we keep the functionality or remove it in the name of symbol
> table minimalism?
>
> Reviewers: mstorsjo, compnerd
>
> Subscribers: hiraditya, compnerd, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D55900
>
> Added:
>     llvm/trunk/test/MC/MachO/file-single.s
> Modified:
>     llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
>     llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>     llvm/trunk/test/MC/COFF/file.s
>
> Modified: llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp?rev=349976&r1=349975&r2=349976&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmInfoCOFF.cpp Fri Dec 21 15:35:48 2018
> @@ -25,7 +25,7 @@ MCAsmInfoCOFF::MCAsmInfoCOFF() {
>    COMMDirectiveAlignmentIsInBytes = false;
>    LCOMMDirectiveAlignmentType = LCOMM::ByteAlignment;
>    HasDotTypeDotSizeDirective = false;
> -  HasSingleParameterDotFile = false;
> +  HasSingleParameterDotFile = true;
>    WeakRefDirective = "\t.weak\t";
>    HasLinkOnceDirective = true;
>
>
> Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=349976&r1=349975&r2=349976&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
> +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Fri Dec 21 15:35:48 2018
> @@ -3360,9 +3360,12 @@ bool AsmParser::parseDirectiveFile(SMLoc
>      }
>    }
>
> -  if (FileNumber == -1)
> +  if (FileNumber == -1) {
> +    if (!getContext().getAsmInfo()->hasSingleParameterDotFile())
> +      return Error(DirectiveLoc,
> +                   "target does not support '.file' without a number");
>      getStreamer().EmitFileDirective(Filename);
> -  else {
> +  } else {
>      // In case there is a -g option as well as debug info from directive
> .file,
>      // we turn off the -g option, directly use the existing debug info
> instead.
>      // Also reset any implicit ".file 0" for the assembler source.
>
> Modified: llvm/trunk/test/MC/COFF/file.s
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/file.s?rev=349976&r1=349975&r2=349976&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/MC/COFF/file.s (original)
> +++ llvm/trunk/test/MC/COFF/file.s Fri Dec 21 15:35:48 2018
> @@ -1,6 +1,11 @@
>  // RUN: llvm-mc -triple i686-windows -filetype obj %s -o - | llvm-objdump
> -t - \
>  // RUN:   | FileCheck %s
>
> +// Round trip through .s output to exercise MCAsmStreamer.
> +// RUN: llvm-mc -triple i686-windows %s -o - \
> +// RUN:   | llvm-mc -triple i686-windows - -filetype=obj -o - |
> llvm-objdump -t - \
> +// RUN:   | FileCheck %s
> +
>  // RUN: llvm-mc -triple i686-windows -filetype obj %s -o - \
>  // RUN:          | llvm-readobj -symbols | FileCheck %s -check-prefix
> CHECK-SCN
>
>
> Added: llvm/trunk/test/MC/MachO/file-single.s
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/MachO/file-single.s?rev=349976&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/MC/MachO/file-single.s (added)
> +++ llvm/trunk/test/MC/MachO/file-single.s Fri Dec 21 15:35:48 2018
> @@ -0,0 +1,8 @@
> +// RUN: not llvm-mc -triple i386-apple-darwin9 %s -o /dev/null 2>&1 |
> FileCheck %s
> +
> +// Previously this crashed MC.
> +
> +// CHECK: error: target does not support '.file' without a number
> +
> +        .file "dir/foo"
> +        nop
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181227/862c41d9/attachment.html>