Possible bug? Undefined behavior in C emitted code?

[angros47]

From this discussion: viewtopic.php?p=300503#p300503

The code:

Code: Select all

Dim shared As long bDimMancala(13)


Sub DrawNumbers()

	For i As Long = 0 To 5
		
		bDimMancala(13 - i) = i
		
	next

End Sub

Produces, in the C intermediate source, the line:

Code: Select all

*(int32*)(((int32)(int32*)BDIMMANCALA$ + (-I$1 << (2 & 31))) + 52) = I$1;

(or (int64), depending on the platform)

The code " (-I$1 << (2 & 31))", specifically, is supposed to produce undefined results, because according to C specifications, the left and right shift operators have a definite behavior only for unsigned numbers (source: https://stackoverflow.com/questions/841 ... d-behavior )

GCC seems not to have problems with that, but clang (used in Emscripten) seems to have issues with it.
Is that a bug of the compiler?

Is the code perhaps supposed to be " (-(I$1 << (2 & 31)))" ?

[adeyblue]

It recently changed to be well defined, so that for instance -1 << 1 = -2 is now guaranteed
https://stackoverflow.com/questions/554 ... t-overflow

So I guess it now depends on how old the compiler. Although I suspect the reason this change was accepted into the standard along with the mandatory use of two's complement is that this is what the overwhelming number of modern C compilers for various architectures were already doing anyway.

TL;DR it didn't used to be technically fine, it's now technically fine, but in practice it's probably always been fine.

[TeeEmCee]

Yes, this is a bug in fbc. Normally I would have expected that bitshifting a negative in this way would just cause a warning, if anything, that could be ignored, but C and C++ compilers do sometimes really punish you hard for technically undefined behaviour.

It recently changed to be well defined, so that for instance -1 << 1 = -2 is now guaranteed
https://stackoverflow.com/questions/554 ... t-overflow

I hadn't heard that C++20 now requires two's complement signed integers. (About time.) However, that's C++. C has made the same change only in C23, which isn't going to be published until next year. As the emscripten problem shows, we can't rely on such a modern standard.

Incidentally there are plenty of interesting changes in C23, such as "variadic functions no longer need a named argument before the ellipsis and the va_start macro no longer needs a second argument", picking up more C++ features such as [[..]] attribute syntax and auto for type inference, native char8_t for UTF-8, and #elifdef and #elifndef convenience preprocessor directives, which is something I very much want to see added to FB.

[angros47]

As the emscripten problem shows, we can't rely on such a modern standard.

Actually, with further investigation, it seems the problem in Emscripten is not caused by that

[TeeEmCee]

Looking at the issue you filed. I'm really surprised to see the bug seems to be in really far down in LLVM. Nice work narrowing down the original problem and reporting it.

But still, should fbc be emitting C code which left-shifts negatives? We might hope to support other compilers aside from GCC and Clang. I have no idea whether it might actually cause a problem anywhere (due to over-aggressive optimisation, rather than due to a hypothetical non-two's complement machine, of course).

[caseih]

The problem with taking advantage of new features in C23 is that most C programs were intended to be portable, and so programmers tend to prefer the lowest common denominator. It's going to take years for them to be widely adopted and supported by the majority of compilers. Although I suppose some esoteric projects like the Linux kernel will adopt these sort of features the quickest.

I expect FBC will be very conservative in the C features the code it emits requires. This even if shifting a negative number were officially defined, I would prefer that FBC emit code that does not require the behavior.

[angros47]

I have no idea whether it might actually cause a problem anywhere (due to over-aggressive optimisation, rather than due to a hypothetical non-two's complement machine, of course).

Neither I have; on the other hand, I know one thing: the C emitter is around since several years, by now, and that specific behavior has never caused issues, as far as I know (the issue with Emscripten, in the end, was unrelated with that: previous versions of FreeBasic used the same trick and it caused no issues with Emscripten). So, I would say "If it's not broken, don't fix it"; at the moment it causes no problems anywhere, and since that behavior is now defined, it means more and more compilers will accept it in the future, so the likelihood of related issues is going to decrease even more in the future

The problem with taking advantage of new features in C23 is that most C programs were intended to be portable, and so programmers tend to prefer the lowest common denominator
......
I would prefer that FBC emit code that does not require the behavior.

Well, GCC and clang are the most portable compilers I have ever seen, since they seem to support almost every modern platform, and a lot of old ones.
Personally, the only situation I can imagine where a different, older compiler might be needed would be to emit code for processors older than 80386, but this would not work anyway because of the different memory model

[caseih]

and since that behavior is now defined, it means more and more compilers will accept it in the future, so the likelihood of related issues is going to decrease even more in the future

It's defined in C++20, but to my knowledge not in ISO C. Am I incorrect? Can you point me at the ISO C standard where this has now been defined?

[angros47]

I thought I read somewhere it was planned to be defined in C 23, but I can't find that page anymore, so I might be wrong

[nehakakar]

From this discussion: viewtopic.php?p=300503#p300503

The code:

Code: Select all

Dim shared As long bDimMancala(13)


Sub DrawNumbers()

	For i As Long = 0 To 5
		
		bDimMancala(13 - i) = i
		
	next

End Sub

Produces, in the C intermediate source, the line:

Code: Select all

*(int32*)(((int32)(int32*)BDIMMANCALA$ + (-I$1 << (2 & 31))) + 52) = I$1;

(or (int64), depending on the platform)

The code " (-I$1 << (2 & 31))", specifically, is supposed to produce undefined results, because according to C specifications, the left and right shift operators have a definite behavior only for unsigned numbers (source: https://stackoverflow.com/questions/841 ... d-behavior )

GCC seems not to have problems with that, but clang (used in Emscripten) seems to have issues with it.
Is that a bug of the compiler?

Is the code perhaps supposed to be " (-(I$1 << (2 & 31)))" ?

You can try this..

Code: Select all

*(int32*)(((int32)(int32*)BDIMMANCALA$ + (-I$1 << 2)) + 52) = I$1;

[coderJeff]

From other topic:
viewtopic.php?p=300547#p300547

It turns out that I saved the previous compiler (1.08) in my archives. And there are no problems with him. Here is the difference in code generation:

1.09:

Code: Select all

fb_IntToStr( (int32)*(int8*)(((int64)(struct $8TMANCALA*)BDIMMANCALA$ + (-(int64)I$2 * 49ll)) + 637ll) );

1.08:

Code: Select all

fb_IntToStr( (int32)*(int8*)(((uint8*)BDIMMANCALA$ + (-I$2 * 49)) + 637) );

So the problem is in the FB compiler.

It was only very recently (with TeeEmCee's help) I was able to start building the test-suite for Emscripten port. And there are still many failures. If can get the test-suite working properly, then should avoid these kinds of changes in future.

Reminder to myself: need to narrow down why this was changed (i.e. find the commits that caused this change instead of just comparing the fbc versions).

[coderJeff]

Forgot all about this. And forget where this was left off. Sorry.

Which post / fragment of fbc code showed the exact problem? Doing a few tests here and the emitted code I'm currently generating isn't quite matching up with the fragments in this thread.

Thanks

[angros47]

The question is for me or for VANYA?

[coderJeff]

The question is for me or for VANYA?

oh, anyone that can answer will be fine with me. I think I found the working folder where I was looking at this issue last but I have absolutely lost track of where I left off with it.