[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

63

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

86

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

571

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 ↗

(On Diff #20955)

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

lib/sanitizer_common/tests/sanitizer_symbolizer_test.cc

25 ↗

(On Diff #20955)

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

45 ↗

(On Diff #20955)

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 *ExtractUptr(const char *str, const char *delims, uptr *result);
+const char *ExtractTokenUpToDelimiter(const char *str, const char *delimiter,
+                                      char **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_posix_libcdep.cc

static const char *DemangleCXXABI(const char *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) {
+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) {
+// 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 = (int)internal_atoll(buff);
+    *result = (uptr)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;
+// Similar to ExtractToken, but looks only for a single delimiter which can
+// be multiple characters long.
+const char *ExtractTokenUpToDelimiter(const char *str, const char *delimiter,
+                                      char **result) {
+  const char *found_delimiter = internal_strstr(str, delimiter);
+  uptr prefix_len =
+      found_delimiter ? found_delimiter - str : internal_strlen(str);
+  *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 += internal_strlen(delimiter);
+  return prefix_end;
 }
 
 class ExternalSymbolizerInterface {
  public:
   // Can't declare pure virtual functions in sanitizer runtimes:
   // __cxa_pure_virtual might be unavailable.
   virtual char *SendCommand(bool is_data, const char *module_name,
                             uptr module_offset) {
while (true) {
 
       AddressInfo *info = &cur->info;
       info->function = function_name;
       // Parse <file>:<line>:<column> buffer.
       char *file_line_info = 0;
       str = ExtractToken(str, "\n", &file_line_info);
       CHECK(file_line_info);
       const char *line_info = ExtractToken(file_line_info, ":", &info->file);
-      line_info = ExtractInt(line_info, ":", &info->line);
-      line_info = ExtractInt(line_info, "", &info->column);
+      uptr line_number;
+      line_info = ExtractUptr(line_info, ":", &line_number);
+      info->line = line_number;
+      if (*line_info != '\0') {
+        uptr column_number;
+        line_info = ExtractUptr(line_info, "", &column_number);
+        info->column = column_number;
+      }
       InternalFree(file_line_info);
 
       // Functions and filenames can be "??", in which case we write 0
       // to address info to mark that names are unknown.
       if (0 == internal_strcmp(info->function, "??")) {
         InternalFree(info->function);
         info->function = 0;
       }