Bug 1304517

(In reply to Adam from comment #0)
> Steps to Reproduce (using my git repo above):
> 1.  git clone https://github.com/eryjus/pascal.git
> 2.  cd pascal
> 3.  make


master pascal$ make
AST    src/pascal.ast
/bin/sh: ast-cc: command not found
LEX    src/lexer.ll
YACC   src/grammar.yy
CC     src/bld-type.cc
src/bld-type.cc:19:21: fatal error: pascal.hh: No such file or directory
compilation terminated.
makefile:96: recipe for target 'obj/bld-type.o' failed
make: *** [obj/bld-type.o] Error 1



Please provide preprocessed source for the file that calls atof and atoi, preferably with both gcc versions.

Created attachment 1120916 [details]
the result of processing pascal.ast through ast-cc

This file should be placed in pascal/include

It will be removed with `make clean` -- please make sure you have a copy to replace it.

Created attachment 1120917 [details]
This is the executable to process pascal.ast to pascal.hh

You can place this file somewhere in your path, or in the pascal folder.

it will process the pascal.ast to pascal.hh as defined in the makefile.



BTW -- My apologies for the miss on the dependency

Created attachment 1120918 [details]
objdump of semant.o created with FC22

This file was created by the FC22 version of the compiler.  Please see the label _ZN7RealLit6semantEv for the start of the calling function.

Created attachment 1120919 [details]
objdump of semant.o created with FC23

This file was created by the FC23 version of the compiler.  Please see the label _ZN7RealLit6semantEv for the start of the calling function.

Created attachment 1120920 [details]
Preprocessed C code from FC22

Created attachment 1120921 [details]
Preprocessed C code from FC23

Please let me know if there is anything else you might need.

Based on a change in my code, it appears that the problem might be rooted in the std::string type.  It also appears that the C++11 standard is now the default for the compiler as of FC23 (which I am not sure is relevant, but is a change).

So, I am looking at the following class [note GetKeyValue() and GetKeyString() methods]:

class Entry {
private:
    static long int _NextID;
    long int ID;
    std::string KeyValue;

public:
    std::string GetKeyValue(void) const { return KeyValue; };
    const char *GetKeyString(void) const { return KeyValue.c_str(); }

public:
    Entry(const std::string &s) : ID(_NextID ++), KeyValue(s) {};

public:
    static std::string strlwr(const std::string &s) { return strlwr(s.c_str()); };
    static std::string strlwr(const char *s);
    bool Equals(const std::string &s) { return (s == KeyValue); };
    std::string GetString(void) const { return KeyValue; }
};


In pascal.hh, I had the following inline method for my RealLit:
    virtual const char *GetString(void) { return entry->GetKeyValue().c_str(); }



When I change the method to:
    virtual const char *GetString(void) { return entry->GetKeyString(); }

.. it works.


Since GetKeyValue() returns a std::string copy of the std::string attribute, I wonder if there are issues with the copy constructor for std::string.....


Finally, when I change the method in the Entry class to:
       std::string &GetKeyValue(void) { return KeyValue; };
.. and use GetKeyValue().c_str(), the results are what I expect.


This, then, also increases my suspicion of the std::string copy constructor.


I will work again on a trivial test case.

(In reply to Adam from comment #9)
> Based on a change in my code, it appears that the problem might be rooted in
> the std::string type.  It also appears that the C++11 standard is now the
> default for the compiler as of FC23 (which I am not sure is relevant, but is
> a change).

No, that's not the case. A new ABI (using the tag "cxx11") is enabled by default, but that's orthogonal to the C++98 vs C++11 standard mode.

The new ABI enables a different definition of std::string, which does not use reference-counting to share the string data between objects.

> In pascal.hh, I had the following inline method for my RealLit:
>     virtual const char *GetString(void) { return
> entry->GetKeyValue().c_str(); }
> 
> 
> 
> When I change the method to:
>     virtual const char *GetString(void) { return entry->GetKeyString(); }
> 
> .. it works.
> 
> 
> Since GetKeyValue() returns a std::string copy of the std::string attribute,
> I wonder if there are issues with the copy constructor for std::string.....

There is no problem with std::string, your code has undefined behaviour. If GetKeyValue returns a copy then it goes out of scope immediately as soon as the function returns and deletes its contents, and the const char* that you get from it by c_str() is a dangling pointer.


> Finally, when I change the method in the Entry class to:
>        std::string &GetKeyValue(void) { return KeyValue; };
> .. and use GetKeyValue().c_str(), the results are what I expect.

Because now the value returned by c_str() is the contents of KeyValue, not some temporary copy of it.

(In reply to Jonathan Wakely from comment #10)
> The new ABI enables a different definition of std::string, which does not
> use reference-counting to share the string data between objects.

You can use the old ABI by defining _GLIBCXX_USE_CXX11_ABI=0 (which is what Fedora 22's compiler does by default).

http://developerblog.redhat.com/2015/02/10/gcc-5-in-fedora/

https://fedoramagazine.org/gcc-5-in-fedora-whats-an-abi-and-what-happens-when-we-change-it/

> > Since GetKeyValue() returns a std::string copy of the std::string attribute,
> > I wonder if there are issues with the copy constructor for std::string.....
> 
> There is no problem with std::string, your code has undefined behaviour. If
> GetKeyValue returns a copy then it goes out of scope immediately as soon as
> the function returns and deletes its contents, and the const char* that you
> get from it by c_str() is a dangling pointer.

N.B. This bug probably would have been revealed by using valgrind.