Ticket #38417

Memory load reordering optimization (-O2) bug

Date d'ouverture: 2018-07-26 22:39 Dernière mise à jour: 2018-07-29 21:08

Rapporteur:
Propriétaire:
(Aucun)
Type:
État:
Atteints
Composant:
Jalon:
(Aucun)
Priorité:
5 - moyen
Sévérité:
5 - moyen
Résolution:
Invalid
Fichier:
Aucun
Vote
Score: 0
No votes
0.0% (0/0)
0.0% (0/0)

Détails

Description

The bug is related to some internal code structure, so tried to reproduce the situation as close as possible. mingw with -O2 optimization flag loads memset-ed value on the first load of atomic (just struct name) value read and initialized value (right value) at the second load call.

Fixes:
* Removing volatile keyword
* Changing int32_t to unsigned long
* Other reduction of code complexity

What is wrong with this code? Is it a bug? GCC7 does not have this issue.

Reproducer

  1. #include <new>
  2. #include <iostream>
  3. #include <cstring>
  4. struct atomic {
  5. unsigned long my_value;
  6. public:
  7. atomic() = default ;
  8. atomic(unsigned long value) : my_value(value) {}
  9. unsigned long load() {
  10. int32_t* v = reinterpret_cast<int32_t*>(&my_value);
  11. return (volatile int32_t&)(*v);
  12. }
  13. };
  14. //--------------------------------------------------------------------------------------------------------
  15. struct TestStruct {
  16. atomic storage;
  17. TestStruct(){
  18. memset(static_cast<void*>(&storage),-1,sizeof(storage));
  19. }
  20. void operator()(unsigned long i){
  21. atomic* ptr2 = new (&storage) atomic(i);
  22. unsigned long val1 = ptr2->load();
  23. std::cout << "Expected:" << i << " Value first: " << val1 << std::endl;
  24. unsigned long val2 = ptr2->load();
  25. std::cout << "Expected:" << i << " Value second: " << val2 << std::endl;
  26. }
  27. };
  28. int main () {
  29. TestStruct()(0L);
  30. //just long also should fail;
  31. return 0;
  32. }

Output:

Expected:0 Value first: 4294967295
Expected:0 Value second: 0

Environment

Windows Server 2016

gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=d:/mingwcompilers/mingw_15.4_gcc_7.3.0/bin/../libexec/gcc/x86_64-w64-mingw32/7.3.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../src/configure --enable-languages=c,c++ --build=x86_64-w64-mingw32 --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --disable-multilib --prefix=/c/temp/gcc/dest --with-sysroot=/c/temp/gcc/dest --disable-libstdcxx-pch --disable-libstdcxx-verbose --disable-nls --disable-shared --disable-win32-registry --with-tune=haswell --enable-threads=posix --enable-libgomp
Thread model: posix
gcc version 7.3.0 (GCC)

ld -v
GNU ld (GNU Binutils) 2.29.1

Ticket History (3/8 Histories)

2018-07-26 22:39 Updated by: nik-ponomarev
  • New Ticket "Memory load reordering optimization (-O2) bug" created
2018-07-27 03:16 Updated by: earnie
  • Composant Update from (Aucun) to OTHER
  • Résolution Update from Aucun to Invalid
  • État Update from Ouvert to Atteints
Commentaire

Invalid Report

  • This is not a mingw.org issue.
  • You should file your report with GCC at https://gcc.gnu.org/bugs/.
  • Your report indicates that you're using a different distribution of GCC than the one we provide.
2018-07-27 19:47 Updated by: nik-ponomarev
Commentaire

Same GCC version does not have this bug

2018-07-27 20:40 Updated by: keith
Commentaire

Reply To nik-ponomarev

Same GCC version does not have this bug

With respect, that's an asinine, and utterly meaningless statement; you are reporting an issue with a specific version of GCC so obviously that particular version of GCC does exhibit the issue.

I'm sorry to say this, but while we thank you for the heads-up, (which will enable us to check whether, or not, our distribution of GCC-7.3 exhibits this issue), we simply cannot help you to resolve your issue, because your report clearly demonstrates that you are not using our GCC distribution; you must seek assistance from the distributor of the compiler you are actually using, (which is not ours), or escalate it upstream to gcc.org. Furthermore, just because some GCC-7.3 build, presumably for some other platform, doesn't exhibit the issue, that does not preclude a bug in target specific gcc.org code, in which case reporting your issue upstream would be entirely appropriate.

2018-07-27 21:15 Updated by: keith
  • Details Updated
2018-07-27 21:19 Updated by: nik-ponomarev
Commentaire

Ok, many thanks for the detailed and informative instructions! I will submit the issue to GCC bug-tracker.

2018-07-28 04:59 Updated by: keith
Commentaire

I don't know exactly what you are trying to demonstrate, with your test case, but if I understand you correctly, you tripped over some unexpected behaviour with std::atomic. FWIW, if I substitute std::atomic<int32_t>, (having also added #include <cstdint>), for your home-brewed alternative struct atomic:

  1. #include <cstdint>
  2. #include <iostream>
  3. #include <cstring>
  4. #include <atomic>
  5. struct TestStruct
  6. { std::atomic<int32_t> storage;
  7. TestStruct(){ memset(static_cast<void *>(&storage),-1,sizeof(storage)); }
  8. void operator()(unsigned long i)
  9. { std::atomic<int32_t>* ptr2 = new (&storage) std::atomic<int32_t>(i);
  10. unsigned long val1 = ptr2->load();
  11. std::cout << "Expected: " << i << "; First load value: " << val1 << std::endl;
  12. unsigned long val2 = ptr2->load();
  13. std::cout << "Expected: " << i << "; Second load value: " << val2 << std::endl;
  14. }
  15. };
  16. int main()
  17. { TestStruct()(0UL);
  18. return 0;
  19. }
Then cross-compile and run it (under wine), with the Linux hosted build of our GCC-7.3.0:
$ mingw32-g++ -O2 --std=gnu++11 t_atomic.cpp
$ ./a.exe
Expected: 0; First load value: 0
Expected: 0; Second load value: 0
Isn't this the behaviour you expect?

BTW, I'm no expert in C++, (in fact, I find that it all too frequently degenerates into inscrutable black magic), doesn't that new operator in my line 11, (corresponding to your line 26), create a memory leak?

2018-07-29 21:08 Updated by: keith
Commentaire

Reply To keith

BTW, I'm no expert in C++, (in fact, I find that it all too frequently degenerates into inscrutable black magic), doesn't that new operator in my line 11, (corresponding to your line 26), create a memory leak?

I guess it doesn't. As I said, I'm no expert in C++, and I'd never seen the "placement" overload of new before. Some research tells me that this usage is simply creating a new pointer, referring to an existing memory "allocation"; there is no newly allocated memory to leak.

Attachment File List

No attachments

Modifier

Please login to add comment to this ticket » Connexion