Seeing a crash with ConstantFP::get

Project Infrastructure LLVM Dev List Archives
#1

Hola LLVMers,

I’m getting a crash when using ConstantFP::get.

I can repro it by adding one line to the Fibonacci example program:

int main(int argc, char **argv) {

int n = argc > 1 ? atol(argv[1]) : 24;

// Create some module to put our function into it.

Module *M = new Module(“test”);

// We are about to create the “fib” function:

Function *FibF = CreateFibFunction(M);

// add the following line

Constant* C = ConstantFP::get(Type::FloatTy,0.0);

Can someone verify this? This is on the PC side of my system which I build with the VStudio files. I don’t know if the Mac side sees this as well, but I’ll try and verify. Seems really odd to have something this basic crash me. It came up because the JITter will call ConstantFP::get with the GenericValue arguments passed it, but the above repro is a lot simpler and is independent of the JITter.

My call stack looks like this:

in here it fails this: assert(category == fcNormal || category == fcNaN);

Fibonacci.exe!llvm::APFloat::significandParts() Line 360 + 0x39 bytes C++

Fibonacci.exe!llvm::APFloat::zeroSignificand() Line 387 + 0x15 bytes C++

Fibonacci.exe!llvm::APFloat::APFloat(const llvm::fltSemantics & ourSemantics={…}, unsigned __int64 value=1) Line 307 C++

Fibonacci.exe!`anonymous namespace’::DenseMapAPFloatKeyInfo::getEmptyKey() Line 277 + 0x11 bytes C++

Fibonacci.exe!llvm::DenseMap<`anonymous namespace’::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP *,A0x5b1fa632::DenseMapAPFloatKeyInfo>::getEmptyKey() Line 236 + 0x9 bytes C++

Fibonacci.exe!llvm::DenseMap<`anonymous namespace’::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP *,A0x5b1fa632::DenseMapAPFloatKeyInfo>::init(unsigned int InitBuckets=64) Line 295 + 0x9 bytes C++

Fibonacci.exe!llvm::DenseMap<anonymous namespace'::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP *,A0x5b1fa632::DenseMapAPFloatKeyInfo>::DenseMap<anonymous namespace’::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP *,A0x5b1fa632::DenseMapAPFloatKeyInfo>(unsigned int NumInitBuckets=64) Line 68 C++

Fibonacci.exe!llvm::ManagedStatic<llvm::DenseMap<`anonymous namespace’::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP *,A0x5b1fa632::DenseMapAPFloatKeyInfo> >::LazyInit() Line 72 + 0x24 bytes C++

Fibonacci.exe!llvm::ManagedStatic<llvm::DenseMap<`anonymous namespace’::DenseMapAPFloatKeyInfo::KeyTy,llvm::ConstantFP ,A0x5b1fa632::DenseMapAPFloatKeyInfo> >::operator() Line 55 C++

Fibonacci.exe!llvm::ConstantFP::get(const llvm::Type * Ty=0x01b352d8, double V=0.00000000000000000) Line 299 + 0xe bytes C++

Fibonacci.exe!main(int argc=1, char * * argv=0x01b35060) Line 99 + 0x13 bytes C++

Fibonacci.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C

Fibonacci.exe!mainCRTStartup() Line 403 C

Thanks,

Chuck.

#2

Hola LLVMers,

I’m getting a crash when using ConstantFP::get.

I can repro it by adding one line to the Fibonacci example program:

int main(int argc, char **argv) {

int n = argc > 1 ? atol(argv[1]) : 24;

// Create some module to put our function into it.

Module *M = new Module(“test”);

// We are about to create the “fib” function:

Function *FibF = CreateFibFunction(M);

// add the following line

Constant* C = ConstantFP::get(Type::FloatTy,0.0);

Can someone verify this? This is on the PC side of my system which I build with the VStudio files. I don’t know if the Mac side sees this as well, but I’ll try and verify. Seems really odd to have something this basic crash me. It came up because the JITter will call ConstantFP::get with the GenericValue arguments passed it, but the above repro is a lot simpler and is independent of the JITter.

My call stack looks like this:

in here it fails this: assert(category == fcNormal || category == fcNaN);

Fibonacci.exe!llvm::APFloat::significandParts() Line 360 + 0x39 bytes C++

Fibonacci.exe!llvm::APFloat::zeroSignificand() Line 387 + 0x15 bytes C++

I’ve been working in this area and probably introduced this, but I don’t see how: zeroSignificand() sets ‘category’ to fcNormal before calling significandParts, so I don’t see how that assert could be triggered. The call stack looks normal for that source line. It doesn’t appear on the Mac. Could your compiler be misoptimizing the code?

#3

It’s in debug. I’m having a look at the assembler it’s producing right now and it’s definitely a little odd for what should be a simple assignment in zeroSignificand.

#4

Hola Dale,

I spent some time walking through what's going on with a friend of mine
from VStudio. Category is given 2 bits in the APFloat class definition.
It's sign extending the enum value for the comparisons when it loads it
out of the class, so the 2 becomes a -2 and the comparison fails. He
sent me a piece of code which I might be able to use to force the issue.
I'll update when I try that out.

Thanks,

Chuck.

#5

OK. This possibility occurred to me, I checked the C++ standard, and the code is valid; this is explicitly required to work, and there’s an example.
However VStudio is what it is. I expect expanding the field to 3 bits is good enough. I don’t think that will hurt anything.

#6

Dale Johannesen wrote:-

>I spent some time walking through what?s going on with a friend of
>mine from VStudio. Category is given 2 bits in the APFloat class
>definition. It?s sign extending the enum value for the comparisons
>when it loads it out of the class, so the 2 becomes a -2 and the
>comparison fails. He sent me a piece of code which I might be able
>to use to force the issue. I?ll update when I try that out.
OK. This possibility occurred to me, I checked the C++ standard, and
the code is valid; this is explicitly required to work, and there's
an example.

Thought this might be my bug for a moment.

4.5p3 right? "If the bit-field has an enumerated type, it is
treated as any other value of that type for promotion purposes".
So the bitfield promotes like the enumeration constant itself and
the comparison should be good. It would be pretty sad indeed if
assigning an enum of that type to the bitfield didn't then compare
equal to itself :slight_smile:

However VStudio is what it is. I expect expanding the field to
3 bits is good enough. I don't think that will hurt anything.

MSVC--.

Neil.

#7

Dale Johannesen wrote:-

I spent some time walking through what?s going on with a friend of
mine from VStudio. Category is given 2 bits in the APFloat class
definition. It?s sign extending the enum value for the comparisons
when it loads it out of the class, so the 2 becomes a -2 and the
comparison fails. He sent me a piece of code which I might be able
to use to force the issue. I?ll update when I try that out.

OK. This possibility occurred to me, I checked the C++ standard, and
the code is valid; this is explicitly required to work, and there's
an example.

Thought this might be my bug for a moment.

4.5p3 right? "If the bit-field has an enumerated type, it is
treated as any other value of that type for promotion purposes".
So the bitfield promotes like the enumeration constant itself and
the comparison should be good.

You might argue with that one if you didn't feel like changing your
compiler, but 9 .6p4 is quite explicit:

... If the value of an enumerator is stored into a bitfield
of the same enumeration type and the number of bits in the bitfield
is large enough to hold all the values of that enumeration type, the
original enumerator value and the value of the bitfield
shall compare equal. [Example:
enum BOOL { f=0, t=1 };
struct A {
BOOL b:1;
};
A a;
void f() {
a.b = t;
if (a.b == t) // shall yield true
{ /* ... */ }
}
—end example]

Looks just like your code.

It would be pretty sad indeed if
assigning an enum of that type to the bitfield didn't then compare
equal to itself :slight_smile:

I agree. So did the committee.

#8

Dale Johannesen wrote:-

You might argue with that one if you didn't feel like changing your
compiler, but 9 .6p4 is quite explicit:

... If the value of an enumerator is stored into a bitfield
of the same enumeration type and the number of bits in the bitfield
is large enough to hold all the values of that enumeration type, the
original enumerator value and the value of the bitfield
shall compare equal. [Example:
enum BOOL { f=0, t=1 };
struct A {
BOOL b:1;
};
A a;
void f() {
a.b = t;
if (a.b == t) // shall yield true
{ /* ... */ }
}
?end example]

Looks just like your code.

Good job finding that, thanks. :slight_smile:

> It would be pretty sad indeed if
> assigning an enum of that type to the bitfield didn't then compare
> equal to itself :slight_smile:

I agree. So did the committee.

One they got right then :slight_smile:

Neil.

#9

Hola Dale,

Bumping it up to 3 bits makes everything work hunky dory on the PC side
of my project. Would you like me to commit that?

Also, after speaking with some VStudio folks, it looks like that issue
will likely remain for VS2k8 as well.

Another trick that was suggested to me was to try something along these
lines:

Enum E : unsigned int { A, B, C, D };

...

Class ... {

E e : 2;

Unsigned int pad : 30;

};

Presumably you could reduce the pad size by switching it to an unsigned
char. If you had 8 bits total worth of enums, presumably they could all
fit into one.

I didn't try that one out since your suggestion was simpler and worked.
:slight_smile:

Chuck.

#10

Hola Dale,

Bumping it up to 3 bits makes everything work hunky dory on the PC side
of my project. Would you like me to commit that?

Also, after speaking with some VStudio folks, it looks like that issue
will likely remain for VS2k8 as well.

I'd prefer to coerce VStudio into standard-conformant behavior, via either flag or bug report.
But it sounds like you've tried. If that won't work, the 3-bit workaround is OK with me
(but this is Neil's code originally). Please include a comment explaining why you're
doing this.

Another trick that was suggested to me was to try something along these
lines:

Enum E : unsigned int { A, B, C, D };

...

Class ... {

E e : 2;

Unsigned int pad : 30;

};

Presumably you could reduce the pad size by switching it to an unsigned
char. If you had 8 bits total worth of enums, presumably they could all
fit into one.

I didn't try that one out since your suggestion was simpler and worked.
:slight_smile:

This sort of thing tends to break when somebody adds more fields. While I don't
think that's likely in APFloat, I consider the 3-bit workaround better.
Thanks for analyzing this.