r224835 - Initialize CodeGeneratorImpl::Ctx in constructor.

[Yaron Keren]

Let's disregard dumb gcc warnings. Initializing pointers only (not value
variables) to nullptr will prevent msan detect uninitialized pointer read,
but will usually/always? core dump instead. Considering msan works on Linux
x64 only, and even then msan isn't used most of the time, how about
creating an LLVM_NULLPTR macro which will expand to nothing on msan builds
and to nullptr on non-msan builds, used like

DIBuilder::DIBuilder(Module &m, bool AllowUnresolvedNodes)
    : M(m), VMContext(M.getContext()), TempEnumTypes(LLVM_NULLPTR),
      TempRetainTypes(LLVM_NULLPTR), TempSubprograms(LLVM_NULLPTR), TempGVs(
LLVM_NULLPTR),
      DeclareFn(LLVM_NULLPTR), ValueFn(LLVM_NULLPTR),
      AllowUnresolvedNodes(AllowUnresolvedNodes) {}



2014-12-26 20:04 GMT+02:00 David Blaikie <dblaikie at gmail.com>:

>
>
> On Thu, Dec 25, 2014 at 9:49 AM, Yaron Keren <yaron.keren at gmail.com>
> wrote:
>
>> msan uninitialized use diagnostic may be more useful to value variables,
>> as nullptr pointer access will core dump without msan. Initializing
>> pointers to nullptr is done everywhere in llvm and clang (search for
>> '(nullptr)')
>>
>
> There are certainly places where initializing to nullptr is necessary
> (because the null value is used/tested before any write occurs). And I
> don't doubt there are many places where it's unnecessary & done out of
> habit/consistency/forward-aovidance of problems (either due to the code
> being written before we had MSan, etc - or because the author wasn't aware
> of the tradeoffs).
>
>
>> such as
>>
>> DIBuilder::DIBuilder(Module &m, bool AllowUnresolvedNodes)
>>     : M(m), VMContext(M.getContext()), TempEnumTypes(nullptr),
>>       TempRetainTypes(nullptr), TempSubprograms(nullptr),
>> TempGVs(nullptr),
>>       DeclareFn(nullptr), ValueFn(nullptr),
>>       AllowUnresolvedNodes(AllowUnresolvedNodes) {}
>>
>> isn't init to nullptr the "right" thing to do?
>>
>
> Not necessarily. This isn't a hard-and-fast rule, but we've certainly
> pushed back on initializing variables that don't need initialization when
> it was motivated by silencing a tool that wasn't smart enough to know that
> these initializations weren't needed (such as GCC's warnings).
>
>
>>
>>
>> 2014-12-25 18:46 GMT+02:00 David Blaikie <dblaikie at gmail.com>:
>>
>>> Any particular reason? It's sometimes handy to leave things
>>> uninitialized if they stents intended to be read until after some other
>>> write - in that way, msan can catch bugs when that constraint is violated.
>>> On Dec 25, 2014 3:41 AM, "Yaron Keren" <yaron.keren at gmail.com> wrote:
>>>
>>>> Author: yrnkrn
>>>> Date: Thu Dec 25 05:38:15 2014
>>>> New Revision: 224835
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=224835&view=rev
>>>> Log:
>>>> Initialize CodeGeneratorImpl::Ctx in constructor.
>>>>
>>>>
>>>> Modified:
>>>>     cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=224835&r1=224834&r2=224835&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Thu Dec 25 05:38:15 2014
>>>> @@ -59,7 +59,7 @@ namespace {
>>>>      CodeGeneratorImpl(DiagnosticsEngine &diags, const std::string&
>>>> ModuleName,
>>>>                        const CodeGenOptions &CGO, llvm::LLVMContext& C,
>>>>                        CoverageSourceInfo *CoverageInfo = nullptr)
>>>> -      : Diags(diags), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
>>>> +      : Diags(diags), Ctx(nullptr), CodeGenOpts(CGO),
>>>> HandlingTopLevelDecls(0),
>>>>          CoverageInfo(CoverageInfo),
>>>>          M(new llvm::Module(ModuleName, C)) {}
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141226/def3cba6/attachment.html>