[PATCH] Add a default constructor to work around Visual Studio compiler bug

[Yung, Douglas]

Hi Yaron,

I think what you suggest is a good programming practice, but that is beyond the scope of this patch. This is meant to be a small fix to an issue we have actually had a problem with.

The test is the best test we could come up with for this issue. At best, it will sometimes fail with the bug still present because the failure depends on whatever value happens to be at the data member's memory location when the object is created which could be 1 or 0. But it does show the failure, so it does exercise the bug.

Douglas Yung

From: Yaron Keren [mailto:yaron.keren at gmail.com]
Sent: Tuesday, February 03, 2015 19:03
To: Yung, Douglas
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Add a default constructor to work around Visual Studio compiler bug

Hi,

Regardless this Visual C++ behaviour vs. C++ rules, a program depending upon the compiler to zero data members in a default constructor looks like a bug, the programmer may have forgotten to write the default constructor and to initialize the values, (which may have been be non-zero!).

The class constructor should be provided explicity and initialize data members to specific known values. So the constructor part of the patch LGTM even without the comments, no need to explain why class data members are initialized in the constructor.

I don't know enough about the test part of the patch.

Yaron



2015-02-04 2:06 GMT+02:00 Yung, Douglas <douglas_yung at playstation.sony.com<mailto:douglas_yung at playstation.sony.com>>:
Ping. Could someone do a review of this small work-around for a Visual Studio bug?

Douglas Yung

From: Yung, Douglas
Sent: Wednesday, January 28, 2015 17:46
To: 'llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>'
Subject: [PATCH] Add a default constructor to work around Visual Studio compiler bug

Hi, I would like to submit the following patch to include/llvm/CodeGen/TargetLoweringObjectFileImpl.h which works around a bug in the Visual Studio compiler.

The bug is that the compiler does not initialize a field in the class when it creates a default constructor when it should be zero-initialized. The result is that when the field is later accessed, the value is whatever value happens to be stored there, instead of the proper zero value (false in this case). The explanation of the issue is the following:

Quoting The C++11 standard section [class.base.init]p8 that "In a non-delegating constructor, if a given non-static data member or base class is not designated by a mem-initializer-id (including the case where there is no mem-initializer-list because the constructor has no ctor-initializer) and the entity is not a virtual base class of an abstract class (10.4), then

- if the entity is a non-static data member that has a brace-or-equal-initializer, the entity is initialized as specified in 8.5;
- otherwise, if the entity is a variant member (9.5), no initialization is performed;
- otherwise, the entity is default-initialized "

The lack of default-initialization is what is causing this problem if we use the Visual Studio compiler:
At CodeGen stage, the implicit default constructor for class TargetLoweringObjectFileELF is called. Class 'TargetLoweringObjectFileELF' has a field called 'UseInitArray'. If that field is set to true, then the compiler will emit global constructors/destructors in the .init_array section. Otherwise, it will emit those symbols in the .ctors sections.

The lack of a user defined default constructor for class TargetLoweringObjectFileELF means that the compiler will have to implicitly generate a default constructor with no initializer and empty body. Then, still according to the standard ( [class.base.init]p8 ) it should default-initialize field 'UseInitArray'.

The Visual Studio compiler does not perform the field initialization. Hence the intermittent problem (depending on the garbage value read from memory we either think we shall emit those symbols on the .ctors section or .init_array section).

To illustrate this issue, I have attached a sample file mytest2.cpp, and the assembly generated for it by the MSVC18 (Visual Studio 2013). The source defines 3 different classes A, B and C. All are identical except for the following characteristics:



*         Class A does not define a default constructor and uses one generated automatically by the compiler

*         Class B defines an empty default constructor

*         Class C defines an empty default constructor which initializes the Boolean field to false

If you examine the assembly generated by the compiler, you can see that the constructors generated for A and B area identical, and C is similar, but it contains an initialization of the Boolean field to false which A and B are both missing.

This patch works around the issue by adding a default constructor to the class which explicitly initializes the field to false (the zero value).

The patch also includes an LLVM test which checks that a .init_array is not generated for a non-linux ELF target.

Douglas Yung

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150208/492a9207/attachment.html>