Bug 1304517 - gcc with libstdc++ -- atof and atoi returning 0.0 and 0 respectively
Summary: gcc with libstdc++ -- atof and atoi returning 0.0 and 0 respectively
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 23
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-02-03 21:25 UTC by Adam
Modified: 2016-02-04 11:07 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-02-04 10:46:51 UTC
Type: Bug


Attachments (Terms of Use)
the result of processing pascal.ast through ast-cc (64.97 KB, text/plain)
2016-02-03 22:16 UTC, Adam
no flags Details
This is the executable to process pascal.ast to pascal.hh (42.53 KB, application/x-executable)
2016-02-03 22:19 UTC, Adam
no flags Details
objdump of semant.o created with FC22 (859.08 KB, text/plain)
2016-02-03 22:34 UTC, Adam
no flags Details
objdump of semant.o created with FC23 (861.89 KB, text/plain)
2016-02-03 22:35 UTC, Adam
no flags Details
Preprocessed C code from FC22 (594.43 KB, text/plain)
2016-02-03 22:43 UTC, Adam
no flags Details
Preprocessed C code from FC23 (589.25 KB, text/plain)
2016-02-03 22:43 UTC, Adam
no flags Details

Description Adam 2016-02-03 21:25:29 UTC
Description of problem:

I have been working on a hobby pascal compiler and after upgrading my Fedora OS to FC23 (from FC22 using dnf), I executed a `make clean` and `make` to refresh my compiler.

I continuing my debugging, I found that atof() was returning 0.0 for strings that were previously working properly in FC22 and atoi() was returning 0 for strings that were previously working in FC22.

I downgraded the gcc (and libstdc++) to FC22 along with the dependencies and checked again and the library functions worked as expected.  I refreshed back to FC23 gcc and libstdc++ and the library functions broke again.  I downgraded again to FC22 and the functions worked as expected.  No code changes were made to my code between the upgrades/downgrades.

I have not been able to duplicate this problem with trivial test cases.  For the moment, the failing code can be found at: https://github.com/eryjus/pascal/tree/d658c6d1af672c26b9508ac9423601f74fe07073

The specific atof() line in question is: https://github.com/eryjus/pascal/blob/d658c6d1af672c26b9508ac9423601f74fe07073/src/semant.cc#L638

I will continue to work on a trivial test case.


Version-Release number of selected component (if applicable):

Installed Packages (working properly)
gcc.x86_64        5.3.1-2.fc22
libstdc++.i686    5.3.1-2.fc22
libstdc++.x86_64  5.3.1-2.fc22

Available Packages (not working)
gcc.x86_64        5.3.1-2.fc23
libstdc++.i686    5.3.1-2.fc23
libstdc++.x86_64  5.3.1-2.fc23


How reproducible:

I can reproduce this every time with the FC23 version of the compiler.



Steps to Reproduce (using my git repo above):
1.  git clone https://github.com/eryjus/pascal.git
2.  cd pascal
3.  make
4.  ./pas-cc --log:semant=8  --log:sym=8 test.pas 2>&1 | less
5.  Find "Converted 3.14159 to a real" & observe the resulting value at the end of the line

Actual results:

atof() evaluates to a 0.0 value
atoi() evaluates to a 0 value


Expected results:

atof() and atoi() should work as they do in the FC22 version of the compiler, as demonstrated by:

Semant(5) [...src/semant.cc:899   ]: Entering NegExpr::semant()
Semant(5) [...src/semant.cc:626   ]: Entering RealLit::semant()
Semant(7) [...src/semant.cc:631   ]: Allocating a constant value structure
Semant(8) [...src/semant.cc:639   ]: Converted 3.14159 to a real number 3.141590
Semant(5) [...src/semant.cc:641   ]: Leaving RealLit::semant()


Additional info:

Comment 1 Jonathan Wakely 2016-02-03 22:10:20 UTC
(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.

Comment 2 Adam 2016-02-03 22:16:28 UTC
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.

Comment 3 Adam 2016-02-03 22:19:39 UTC
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

Comment 4 Adam 2016-02-03 22:34:08 UTC
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.

Comment 5 Adam 2016-02-03 22:35:22 UTC
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.

Comment 6 Adam 2016-02-03 22:43:37 UTC
Created attachment 1120920 [details]
Preprocessed C code from FC22

Comment 7 Adam 2016-02-03 22:43:54 UTC
Created attachment 1120921 [details]
Preprocessed C code from FC23

Comment 8 Adam 2016-02-03 22:47:23 UTC
Please let me know if there is anything else you might need.

Comment 9 Adam 2016-02-04 02:30:58 UTC
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.

Comment 10 Jonathan Wakely 2016-02-04 10:46:51 UTC
(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.

Comment 11 Jonathan Wakely 2016-02-04 11:07:38 UTC
(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.


Note You need to log in before you can comment on or make changes to this bug.