[compiler-rt] Symbolizer refactoring: ExtractToken and friends

[kubamracek]

Summary

Part of http://reviews.llvm.org/D7827. Removes ExtractInt and only leaves ExtractUptr. Makes these functions available in the sanitizer_symbolizer.h header. Adds ExtractTokenUpToDelimiter, which is currently unused, but I plan to use it for AtosSymbolizer later, and since it's related, I included it in this patch.

[samsonov]

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

57–58

Can these function go to sanitizer_symbolizer.cc (or sanitizer_symbolizer_libcdep.cc)? They don't seem to be linux-specific.

59

Please add some unit tests for these functions to sanitizer_common/tests

543

Please don't do this. If you want to remove ExtractInt in favor of ExtractUptr, please turn AddressInfo::line and AddressInfo::column into uptr in a separate change (and audit all cases where we use/print them). Another option is to have single ExtractNumber<T>, but then we'd need to provide definition for this function in a header, and I wouldn't like to include heavy sanitizer_allocator_internal.h in sanitizer_symbolizer.h

[kubamracek]

Updating patch. Let's start with moving the functions, making them available in the header and adding tests.

[samsonov]

LGTM, but please address comments.

lib/sanitizer_common/sanitizer_symbolizer.h

27

Consider declaring these functions in sanitizer_symbolizer_internal.h instead (after you create it).

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

20

Remove comments, now that you have them in a header.

lib/sanitizer_common/tests/sanitizer_symbolizer_test.cc

25

Here and below - "expected" result should go first, actual result should go second.

45

E.g. this line should be

EXPECT_EQ(123U, token);

[kubamracek]

Landed in r231027.

lib/sanitizer_common/sanitizer_symbolizer.h

 #ifndef SANITIZER_SYMBOLIZER_H
 #define SANITIZER_SYMBOLIZER_H
 
 #include "sanitizer_common.h"
 #include "sanitizer_mutex.h"
 
 namespace __sanitizer {
 
+// Parsing helpers, 'str' is searched for delimiter(s) and a string or uptr
+// is extracted. When extracting a string, a newly allocated (using
+// InternalAlloc) and null-terminataed buffer is returned. They return a pointer
+// to the next characted after the found delimiter.
+const char *ExtractToken(const char *str, const char *delims, char **result);
+const char *ExtractInt(const char *str, const char *delims, int *result);
+const char *ExtractUptr(const char *str, const char *delims, uptr *result);
+
 struct AddressInfo {
   // Owns all the string members. Storage for them is
   // (de)allocated using sanitizer internal allocator.
   uptr address;
 
   char *module;
   uptr module_offset;
 

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

 //===-- sanitizer_symbolizer_libcdep.cc -----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is shared between AddressSanitizer and ThreadSanitizer
 // run-time libraries.
 //===----------------------------------------------------------------------===//
 
+#include "sanitizer_allocator_internal.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_symbolizer.h"
 
 namespace __sanitizer {
 
+// Extracts the prefix of "str" that consists of any characters not
+// present in "delims" string, and copies this prefix to "result", allocating
+// space for it.
+// Returns a pointer to "str" after skipping extracted prefix and first
+// delimiter char.
+const char *ExtractToken(const char *str, const char *delims, char **result) {
+  uptr prefix_len = internal_strcspn(str, delims);
+  *result = (char*)InternalAlloc(prefix_len + 1);
+  internal_memcpy(*result, str, prefix_len);
+  (*result)[prefix_len] = '\0';
+  const char *prefix_end = str + prefix_len;
+  if (*prefix_end != '\0') prefix_end++;
+  return prefix_end;
+}
+
+// Same as ExtractToken, but converts extracted token to integer.
+const char *ExtractInt(const char *str, const char *delims, int *result) {
+  char *buff;
+  const char *ret = ExtractToken(str, delims, &buff);
+  if (buff != 0) {
+    *result = (int)internal_atoll(buff);
+  }
+  InternalFree(buff);
+  return ret;
+}
+
+// Same as ExtractToken, but converts extracted token to uptr.
+const char *ExtractUptr(const char *str, const char *delims, uptr *result) {
+  char *buff;
+  const char *ret = ExtractToken(str, delims, &buff);
+  if (buff != 0) {
+    *result = (uptr)internal_atoll(buff);
+  }
+  InternalFree(buff);
+  return ret;
+}
+
 Symbolizer *Symbolizer::GetOrInit() {
   SpinMutexLock l(&init_mu_);
   if (symbolizer_)
     return symbolizer_;
   if ((symbolizer_ = PlatformInit()))
     return symbolizer_;
   return Disable();
 }
 
 }  // namespace __sanitizer

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

static const char *DemangleCXXABI(const char *name) {
   // the returned value.
   if (__cxxabiv1::__cxa_demangle)
     if (const char *demangled_name =
           __cxxabiv1::__cxa_demangle(name, 0, 0, 0))
       return demangled_name;
 
   return name;
 }
 
-// Extracts the prefix of "str" that consists of any characters not
-// present in "delims" string, and copies this prefix to "result", allocating
-// space for it.
-// Returns a pointer to "str" after skipping extracted prefix and first
-// delimiter char.
-static const char *ExtractToken(const char *str, const char *delims,
-                                char **result) {
-  uptr prefix_len = internal_strcspn(str, delims);
-  *result = (char*)InternalAlloc(prefix_len + 1);
-  internal_memcpy(*result, str, prefix_len);
-  (*result)[prefix_len] = '\0';
-  const char *prefix_end = str + prefix_len;
-  if (*prefix_end != '\0') prefix_end++;
-  return prefix_end;
-}
-
-// Same as ExtractToken, but converts extracted token to integer.
-static const char *ExtractInt(const char *str, const char *delims,
-                              int *result) {
-  char *buff;
-  const char *ret = ExtractToken(str, delims, &buff);
-  if (buff != 0) {
-    *result = (int)internal_atoll(buff);
-  }
-  InternalFree(buff);
-  return ret;
-}
-
-static const char *ExtractUptr(const char *str, const char *delims,
-                               uptr *result) {
-  char *buff;
-  const char *ret = ExtractToken(str, delims, &buff);
-  if (buff != 0) {
-    *result = (uptr)internal_atoll(buff);
-  }
-  InternalFree(buff);
-  return ret;
-}
-
 SymbolizerProcess::SymbolizerProcess(const char *path)
     : path_(path),
       input_fd_(kInvalidFd),
       output_fd_(kInvalidFd),
       times_restarted_(0),
       failed_to_start_(false),
       reported_invalid_path_(false) {
   CHECK(path_);
   CHECK_NE(path_[0], '\0');
 }
if (libbacktrace_symbolizer_ != 0) {
       mu_.CheckLocked();
       if (SymbolizedStack *res = libbacktrace_symbolizer_->SymbolizeCode(
               addr, module_name, module_offset))
         return res;
     }
     // Always fill data about module name and offset.
     SymbolizedStack *res = SymbolizedStack::New(addr);
     res->info.FillAddressAndModuleInfo(addr, module_name, module_offset);
     if (SymbolizerTool *tool = GetSymbolizerTool()) {
       SymbolizerScope sym_scope(this);
       tool->SymbolizePC(addr, res);
     }
     return res;
   }
 
   bool SymbolizeData(uptr addr, DataInfo *info) override {
     BlockingMutexLock l(&mu_);

lib/sanitizer_common/tests/CMakeLists.txt

set(SANITIZER_UNITTESTS
   sanitizer_posix_test.cc
   sanitizer_printf_test.cc
   sanitizer_procmaps_test.cc
   sanitizer_stackdepot_test.cc
   sanitizer_stacktrace_printer_test.cc
   sanitizer_stacktrace_test.cc
   sanitizer_stoptheworld_test.cc
   sanitizer_suppressions_test.cc
+  sanitizer_symbolizer_test.cc
   sanitizer_test_main.cc
   sanitizer_thread_registry_test.cc)
 
 set(SANITIZER_TEST_HEADERS
   sanitizer_pthread_wrappers.h
   sanitizer_test_config.h
   sanitizer_test_utils.h)
 foreach(header ${SANITIZER_HEADERS})

lib/sanitizer_common/tests/sanitizer_symbolizer_test.cc

+//===-- sanitizer_symbolizer_test.cc --------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Tests for sanitizer_symbolizer.h
+//
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_common/sanitizer_allocator_internal.h"
+#include "sanitizer_common/sanitizer_symbolizer.h"
+#include "gtest/gtest.h"
+
+namespace __sanitizer {
+
+TEST(Symbolizer, ExtractToken) {
+  char *token;
+  const char *rest;
+  
+  rest = ExtractToken("a;b;c", ";", &token);
+  EXPECT_STREQ(token, "a");
+  EXPECT_STREQ(rest, "b;c");
+  InternalFree(token);
+
+  rest = ExtractToken("aaa-bbb.ccc", ";.-*", &token);
+  EXPECT_STREQ(token, "aaa");
+  EXPECT_STREQ(rest, "bbb.ccc");
+  InternalFree(token);
+}
+
+TEST(Symbolizer, ExtractInt) {
+  int token;
+  const char *rest = ExtractInt("123,456;789", ";,", &token);
+  EXPECT_EQ(token, (int)123);
+  EXPECT_STREQ(rest, "456;789");
+}
+
+TEST(Symbolizer, ExtractUptr) {
+  uptr token;
+  const char *rest = ExtractUptr("123,456;789", ";,", &token);
+  EXPECT_EQ(token, (uptr)123);
+  EXPECT_STREQ(rest, "456;789");
+}
+
+}  // namespace __sanitizer