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

[Alexey Samsonov via cfe-commits]

On Mon, Mar 14, 2016 at 8:55 AM, Evgenii Stepanov via cfe-commits <
cfe-commits at lists.llvm.org> 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.
>

Right, but it was a while ago, and it's a shame it wasn't completed
properly :-/
I've reopened https://llvm.org/bugs/show_bug.cgi?id=17303, but I think the
changes
were abandoned long before Kostya's last comment in that bug.

IIRC the approach for "trimming" debug info used to implement remarks is
way too
clumsy for ASan: we just delete the link to the root of DIE tree if
necessary, thus dropping all the
debug info. "Lowering" the amount of emitted debug info from full debug
info (that would contain names for
local variables ASan/MSan is interested in) to -gline-tables-only in the
backend would be harder.


>
> >>>
> >>> 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
> >>>
> >>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160314/f5268e38/attachment.html>