r263429 - [Frontend] Disable value name discarding for all sanitizers.

[David Blaikie via cfe-commits]

On Mon, Mar 14, 2016 at 8:55 AM, Evgenii Stepanov <eugenis at google.com>
wrote:

> On Mon, Mar 14, 2016 at 8:48 AM, Benjamin Kramer <benny.kra at gmail.com>
> wrote:
> > On Mon, Mar 14, 2016 at 3:59 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> Yeah - how are they relying on them in a non-asserts build anyway?
> (were we
> >> naming certain things regardless of +/-Asserts? (well, I know we were
> naming
> >> some things, but mostly down in LLVM, I thought Clang generally used the
> >> IRBuilder & thus didn't name things in non-Asserts builds))
> >
> > Allocas used to always have names as a debugging feature, see r246991.
> >
> >> On Mon, Mar 14, 2016 at 6:36 AM, Chandler Carruth via cfe-commits
> >> <cfe-commits at lists.llvm.org> wrote:
> >>>
> >>> A long time ago I argued that the sanitizers should be using debug info
> >>> instead of alloca names for this, and I really thought that they had
> >>> implemented this... Is that not the case?
> >>>
> >>> It is really bad that the sanitizers are relying on this stuff...
>
> We've never implemented this though I think Alexey looked into it at
> one point. We'd need to refactor the handling of debug options
> (-g/-gline-tables-only) so that debug metadata is emitted with ASan
> even without -g and then discarded in the backend.
>

We have some precedent for this with the optimizer remark functionality, if
I recall correctly. Might work for non-gmlt-like stuff too.


>
> >>>
> >>> On Mon, Mar 14, 2016 at 2:28 PM Benjamin Kramer via cfe-commits
> >>> <cfe-commits at lists.llvm.org> wrote:
> >>>>
> >>>> Author: d0k
> >>>> Date: Mon Mar 14 08:23:58 2016
> >>>> New Revision: 263429
> >>>>
> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=263429&view=rev
> >>>> Log:
> >>>> [Frontend] Disable value name discarding for all sanitizers.
> >>>>
> >>>> ASan also relies on names on allocas and will emit unhelpful output if
> >>>> they're not present. Just force-enable value names for now. Should
> >>>> unbreak release builds of asan.
> >>>>
> >>>> Modified:
> >>>>     cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >>>>
> >>>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> >>>> URL:
> >>>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=263429&r1=263428&r2=263429&view=diff
> >>>>
> >>>>
> ==============================================================================
> >>>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> >>>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Mar 14 08:23:58
> >>>> 2016
> >>>> @@ -541,6 +541,7 @@ static bool ParseCodeGenArgs(CodeGenOpti
> >>>>    Opts.DisableFPElim =
> >>>>        (Args.hasArg(OPT_mdisable_fp_elim) || Args.hasArg(OPT_pg));
> >>>>    Opts.DisableFree = Args.hasArg(OPT_disable_free);
> >>>> +  Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
> >>>>    Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
> >>>>    Opts.FloatABI = Args.getLastArgValue(OPT_mfloat_abi);
> >>>>    if (Arg *A = Args.getLastArg(OPT_meabi)) {
> >>>> @@ -793,12 +794,6 @@ static bool ParseCodeGenArgs(CodeGenOpti
> >>>>    Opts.CudaGpuBinaryFileNames =
> >>>>        Args.getAllArgValues(OPT_fcuda_include_gpubinary);
> >>>>
> >>>> -  // DiscardValueNames is set here so that it can depend (perhaps
> >>>> temporarily)
> >>>> -  // on other options.
> >>>> -  // We check SanitizeMemoryTrackOrigins here because the backend
> pass
> >>>> depends
> >>>> -  // on the name of the alloca in order to print out names.
> >>>> -  Opts.DiscardValueNames =
> >>>> -      Args.hasArg(OPT_discard_value_names) &&
> >>>> !Opts.SanitizeMemoryTrackOrigins;
> >>>>    return Success;
> >>>>  }
> >>>>
> >>>> @@ -2158,6 +2153,12 @@ bool CompilerInvocation::CreateFromArgs(
> >>>>      if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC)
> >>>>        Res.getLangOpts()->ObjCExceptions = 1;
> >>>>    }
> >>>> +
> >>>> +  // FIXME: Override value name discarding when sanitizers are used
> >>>> because the
> >>>> +  // backend passes depend on the name of the alloca in order to
> print
> >>>> out
> >>>> +  // names.
> >>>> +  Res.getCodeGenOpts().DiscardValueNames &=
> >>>> Res.getLangOpts()->Sanitize.empty();
> >>>> +
> >>>>    // FIXME: ParsePreprocessorArgs uses the FileManager to read the
> >>>> contents of
> >>>>    // PCH file and find the original header name. Remove the need to
> do
> >>>> that in
> >>>>    // ParsePreprocessorArgs and remove the FileManager
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> cfe-commits mailing list
> >>>> cfe-commits at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>>
> >>>
> >>> _______________________________________________
> >>> cfe-commits mailing list
> >>> cfe-commits at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160314/5c2612a0/attachment-0001.html>