On 04/04/14 11:30, Reza Jelveh wrote:
> On 04/04/14 02:14, Dong, Eric wrote:
>> This error info is printed in CVfrCompiler::Compile () function.
>> And before call this function, if vfrcompiler parse the code
>> success, the status will be set to STATUS_PREPROCESSED, else it
>> will be set to STATUS_DEAD. I don't think it's necessary to initial
>> the value as STATUS_STARTED.

Yes, it is necessary. See below.

>>
>> Also I'm curious what's the root cause for your error and does this patch 
>> fix your error?
> 
> Hey, sorry I just removed all my trace notes yesterday. As far as I remember
> it never makes it to STATUS_INITIALIZED. I'm not sure what kind of bug that
> is, because i tried both gcc 4.4 and 4.8 in archlinux. So it must be some
> other environment based issue.
> 
> Anyway, yes the patch fixes it. I found two other people having the same issue
> here: https://aur.archlinux.org/packages/ovmf-svn/

Reza, thank you for tracking this down; and indeed there's a bug in
VfrCompiler.

The problem is that the CVfrCompiler::mRunStatus member, a non-static,
POD (plain old data) member, is not initialized. (COMPILER_RUN_STATUS is
an enum type.)

Hence in the following (otherwise successful) call chain, mRunStatus has
indeterminate value, causing undefined behavior very early:

CVfrCompiler::CVfrCompiler()
  CVfrCompiler::OptionInitialization() // doesn't set mRunStatus
  CVfrCompiler::IS_RUN_STATUS()
    // reads mRunStatus

Here's why CVfrCompiler::mRunStatus is not initialized, according to the
C++03 standard (I could use a C++11 draft too, but I don't think
BaseTools are built in C++11 mode):

--------*--------
12     Special member functions
12.6   Initialization
12.6.2 Initializing bases and members

  4  If a given nonstatic data member or base class is not named by a
     mem-initializer-id (including the case where there is no
     mem-initializer-list because the constructor has no
     ctor-initializer), then

     — If the entity is a nonstatic data member of (possibly
       cv-qualified) class type (or array thereof) or a base class, and
       the entity class is a non-POD class, the entity is default-
       initialized (8.5). If the entity is a nonstatic data member of a
       const-qualified type, the entity class shall have a
       user-declared default constructor.

     — Otherwise, the entity is not initialized. If the entity is of
       const-qualified type or reference type, or of a (possibly
       cv-qualified) POD class type (or array thereof) containing
       (directly or indirectly) a member of a const-qualified type, the
       program is ill-formed.

     After the call to a constructor for class X has completed, if a
     member of X is neither specified in the constructor’s
     mem-initializers, nor default-initialized, nor value-initialized,
     nor given a value during execution of the body of the constructor,
     the member has indeterminate value.
--------*--------

In this case:
(a) CVfrCompiler::CVfrCompiler() is the constructor,
(b) mRunStatus is a nonstatic data member,
(c) the constructor has no ctor-initializer; ie. a member initializer
    list, separated with a colon (:), as in

    CVfrCompiler::CVfrCompiler (
      IN INT32      Argc,
      IN CHAR8      **Argv
      ) : mRunStatus (STATUS_STARTED)
    {
      // ...
    }

(d) mRunStatus has an enumeration type, ie. a POD type; hence the
    second bullet applies
(e) The ill-formedness part of the 2nd bullet doesn't apply.

(The program is not ill-formed, but the read access to mRunStatus is
undefined.)

I think that Reza's fix is correct, I'd just put the

  SET_RUN_STATUS (STATUS_STARTED);

statement right at the top of the constructor, instead of pushing it
down to CVfrCompiler::OptionInitialization().

(Another possibility would be to use a ctor-initializer, like in (c)
above, but I think that the BaseTools coding style doesn't allow that.)

Thanks
Laszlo

------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to