MengRao / fmtlog

fmtlog is a performant fmtlib-style logging library with latency in nanoseconds.
MIT License
807 stars 124 forks source link

undefined behaviour found by asan (llvm) #20

Closed matthijs closed 2 years ago

matthijs commented 2 years ago

Hi,

When running the following small program under the memory sanitizers:

#include "fmtlog/fmtlog.h"

int main(int argc, char** argv)
{
  fmtlog::startPollingThread();
  logi("some data: {}", argc);
  fmtlog::stopPollingThread();
  return 0;
}

I got the following output:

/home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17: runtime error: store to misaligned address 0x000000f28e77 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
0x000000f28e77: note: pointer points here
 30 32 31 2d 00  00 2d 00 00 20 00 00 3a  00 00 3a 00 00 2e 00 00  00 00 00 00 00 00 00 00  00 00 00
             ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17 in
/home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17: runtime error: store to misaligned address 0x000000f28e83 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
0x000000f28e83: note: pointer points here
 3a  00 00 3a 00 00 2e 30 31  31 35 36 38 30 35 37 00  00 00 00 00 00 00 00 00  00 68 35 5b 3c 7f 00
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17 in
/home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17: runtime error: store to misaligned address 0x000000f28e7d for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
0x000000f28e7d: note: pointer points here
 2d 30 39 20 00 00 3a  33 33 3a 31 36 2e 30 31  31 35 36 38 30 35 37 00  00 00 00 00 00 00 00 00  00
             ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/matthijs/t5/fmtlog/fmtlog-inl.h:72:17 in
22:33:16.011568 main.cpp:6       INF[4743  ] some data: 1

Compile line:

export ASAN_SYMBOLIZER_PATH="/usr/lib/llvm-12/bin/llvm-symbolizer"
export ASAN_OPTIONS="check_initialization_order=1:detect_stack_use_after_return=1:strict_string_checks=1:strict_init_order=1"
export LSAN_OPTIONS="use_unaligned=1"
/usr/bin/clang++-12 -DFMTLOG_HEADER_ONLY -DFMT_HEADER_ONLY -fno-omit-frame-pointer -O1 -fsanitize=address,undefined,pointer-compare,pointer-subtract,leak -fsanitize-address-use-after-scope -stdlib=libc++ -g -std=gnu++17 -o main main.cpp
./main

I am not sure if the sanitizers are to strict here or there is actual 'undefined behaviour' here. I also didn't take a look at the code on whats happening here...

MengRao commented 2 years ago

It's not a problem for X86 arch, you can refer to the discussion on a similar issue: https://github.com/openwall/john/issues/1225.

matthijs commented 2 years ago

Thanks, I've added instrumentation to prevent the warning it generates.

aacirino commented 2 years ago

Hi @matthijs, how exactly did you solve this issue?

matthijs commented 2 years ago

Hi @aacirino

I am using the clang compiler and applied the diff below. It disables the 'undefined' sanitizer from clang for the fromi template function.

diff --git a/fmtlog/fmtlog-inl.h b/fmtlog/fmtlog-inl.h
index bb13719..f9e116d 100644
--- a/fmtlog/fmtlog-inl.h
+++ b/fmtlog/fmtlog-inl.h
@@ -57,9 +57,6 @@ public:
     char operator[](int i) const { return s[i]; }

     template<typename T>
+#if defined(__clang__)
+    __attribute__((no_sanitize("undefined")))
+#endif
     void fromi(T num) {
       if constexpr (Size & 1) {
         s[Size - 1] = '0' + (num % 10);
aacirino commented 2 years ago

Hi @aacirino

I am using the clang compiler and applied the diff below. It disables the 'undefined' sanitizer from clang for the fromi template function.

Thank you very much.