Clean up compile-time warnings (gcc 7.1) #10

TheBlueMatt · 2017-07-12T23:32:46Z

max_file_size was already a size_t, so return that.
ecx was somehow being listed as used-uninitialized

sipa · 2017-07-12T23:35:10Z

db/memtable.cc

@@ -101,7 +101,7 @@ void MemTable::Add(SequenceNumber s, ValueType type,
  p += 8;
  p = EncodeVarint32(p, val_size);
  memcpy(p, value.data(), val_size);
-  assert((p + val_size) - buf == encoded_len);
+  assert((p + val_size) - buf >= 0 && size_t((p + val_size) - buf) == encoded_len);


Alternative: assert(p + val_size == buf + encoded_len); ?

sipa · 2017-07-19T03:53:49Z

utACK 64b378c

gmaxwell · 2017-07-21T07:50:59Z

port/port_posix_sse.cc

@@ -54,7 +54,7 @@ static inline bool HaveSSE42() {
  __cpuid(cpu_info, 1);
  return (cpu_info[2] & (1 << 20)) != 0;
 #elif defined(__GNUC__)
-  unsigned int eax, ebx, ecx, edx;
+  unsigned int eax, ebx, ecx = 0, edx;
  __get_cpuid(1, &eax, &ebx, &ecx, &edx);
  return (ecx & (1 << 20)) != 0;


It's concerning if the compiler is emitting a use without init here, suggests that it may be miscompiling the code.

__get_cpuid can fail if the function parameter isn't supported by the hardware (which won't be the case for any supported CPU), in which case it does not modify eax/ebx/ecx/edx.

ah bingo. @TheBlueMatt see how the sse4 stuff in bitcoin master uses __get_cpuid. please do that.

gmaxwell

ACK but the use without init really deserves a triple check, e.g. that the flags on the asm aren't set wrong in a way that makes it unable to tell that the asm that macro expands to modifies memory.

Edit: With nit.

sipa · 2017-07-21T21:42:26Z

Needs rebase; also see my comment on __get_cpuid not updating output parameters.

TheBlueMatt · 2017-07-23T18:06:03Z

OK, stripped out the cpuid check as it was fixed in #5 (though not in the same way that was fixed in Core, in the same way that I had done here, I dont think it matters so I'll let someone else change it if they want).

sipa · 2017-07-23T18:10:09Z

db/memtable.cc

@@ -101,7 +101,7 @@ void MemTable::Add(SequenceNumber s, ValueType type,
  p += 8;
  p = EncodeVarint32(p, val_size);
  memcpy(p, value.data(), val_size);
-  assert((p + val_size) - buf == encoded_len);
+  assert(size_t((p + val_size)) == encoded_len + size_t(buf));


No need for any casts here I think; it's just pointer arithmetic now.

Hmm, indeed, sorry, changed the ordering around to make it mroe clear.

* max_file_size was already a size_t, so return that.

sipa · 2017-07-31T06:40:23Z

ACK 0ec2a34 (ran the LevelDB unit tests)

0ec2a34 Clean up compile-time warnings (gcc 7.1) (Matt Corallo) Pull request description: * max_file_size was already a size_t, so return that. * ecx was somehow being listed as used-uninitialized Tree-SHA512: 6aeb9d6ce343d27c00338a379e6f359a6591e06fda978204133b9f81d817d99d4e4fcb7c851cf366276ef0171cfe77fa5765d836014dd6f213653ac53420d121

maflcko · 2017-07-31T07:20:46Z

@TheBlueMatt Low priority, but you might want to consider submitting this upstream as well.

b13a68e Squashed 'src/leveldb/' changes from 196962f..c521b3a (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315

b13a68e Squashed 'src/leveldb/' changes from 196962ff0..c521b3ac6 (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields)

b13a68e Squashed 'src/leveldb/' changes from 196962ff0..c521b3ac6 (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315

Summary: Includes: bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from Bitcoin-ABC#5 (Cory Fields) Removes many warnings on MacOSX of the form: In file included from leveldb/util/cache.cc:10: In file included from ./leveldb/port/port.h:14: In file included from ./leveldb/port/port_posix.h:47: ./leveldb/port/atomic_pointer.h:55:3: warning: 'OSMemoryBarrier' is deprecated: first deprecated in macOS 10.12 - Use std::atomic_thread_fence() from <atomic> instead [-Wdeprecated-declarations] OSMemoryBarrier(); ^ The bitcoin core patches have been included in BU already by sickpig. Test Plan: make check. Installed and running fine. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D584

Summary: Includes: bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Removes many warnings on MacOSX of the form: In file included from leveldb/util/cache.cc:10: In file included from ./leveldb/port/port.h:14: In file included from ./leveldb/port/port_posix.h:47: ./leveldb/port/atomic_pointer.h:55:3: warning: 'OSMemoryBarrier' is deprecated: first deprecated in macOS 10.12 - Use std::atomic_thread_fence() from <atomic> instead [-Wdeprecated-declarations] OSMemoryBarrier(); ^ The bitcoin core patches have been included in BU already by sickpig. Test Plan: make check. Installed and running fine. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D584

b13a68e Squashed 'src/leveldb/' changes from 196962f..c521b3a (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315

b13a68e Squashed 'src/leveldb/' changes from 196962ff0..c521b3ac6 (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315

b13a68e Squashed 'src/leveldb/' changes from 196962f..c521b3a (Pieter Wuille) Pull request description: Includes: * bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille) * bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields) * bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis) * bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo) * bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from #5 (Cory Fields) Tree-SHA512: 2b88a99a86ed8c74c860de13a123ea7f5424d35d314be564820cf83aaae8308383403f7cd56f17c241cfee4885699796141fed666559c21044eaabaeea073315

sipa reviewed Jul 12, 2017

View reviewed changes

TheBlueMatt force-pushed the bitcoin-fork branch from 3a18f15 to 64b378c Compare July 14, 2017 23:35

gmaxwell reviewed Jul 21, 2017

View reviewed changes

gmaxwell approved these changes Jul 21, 2017

View reviewed changes

TheBlueMatt force-pushed the bitcoin-fork branch from 64b378c to 50f24f1 Compare July 23, 2017 18:05

sipa reviewed Jul 23, 2017

View reviewed changes

Clean up compile-time warnings (gcc 7.1)

0ec2a34

* max_file_size was already a size_t, so return that.

TheBlueMatt force-pushed the bitcoin-fork branch from 50f24f1 to 0ec2a34 Compare July 23, 2017 18:55

sipa merged commit 0ec2a34 into bitcoin-core:bitcoin-fork Jul 31, 2017

sipa mentioned this pull request Jul 31, 2017

Update to latest Bitcoin patches for LevelDB bitcoin/bitcoin#10958

Merged

sickpig mentioned this pull request Aug 17, 2017

[port] Further LevelDB ports BitcoinUnlimited/BitcoinUnlimited#755

Merged

maflcko mentioned this pull request Nov 6, 2019

Update to upstream (1.22+) #25

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up compile-time warnings (gcc 7.1) #10

Clean up compile-time warnings (gcc 7.1) #10

TheBlueMatt commented Jul 12, 2017

sipa Jul 12, 2017

TheBlueMatt Jul 14, 2017

sipa commented Jul 19, 2017

gmaxwell Jul 21, 2017

sipa Jul 21, 2017

gmaxwell Jul 22, 2017

gmaxwell left a comment •

edited

Loading

sipa commented Jul 21, 2017 •

edited

Loading

TheBlueMatt commented Jul 23, 2017

sipa Jul 23, 2017

TheBlueMatt Jul 23, 2017

sipa commented Jul 31, 2017 •

edited

Loading

maflcko commented Jul 31, 2017

Clean up compile-time warnings (gcc 7.1) #10

Clean up compile-time warnings (gcc 7.1) #10

Conversation

TheBlueMatt commented Jul 12, 2017

sipa Jul 12, 2017

Choose a reason for hiding this comment

TheBlueMatt Jul 14, 2017

Choose a reason for hiding this comment

sipa commented Jul 19, 2017

gmaxwell Jul 21, 2017

Choose a reason for hiding this comment

sipa Jul 21, 2017

Choose a reason for hiding this comment

gmaxwell Jul 22, 2017

Choose a reason for hiding this comment

gmaxwell left a comment • edited Loading

Choose a reason for hiding this comment

sipa commented Jul 21, 2017 • edited Loading

TheBlueMatt commented Jul 23, 2017

sipa Jul 23, 2017

Choose a reason for hiding this comment

TheBlueMatt Jul 23, 2017

Choose a reason for hiding this comment

sipa commented Jul 31, 2017 • edited Loading

maflcko commented Jul 31, 2017

gmaxwell left a comment •

edited

Loading

sipa commented Jul 21, 2017 •

edited

Loading

sipa commented Jul 31, 2017 •

edited

Loading