asan_interceptors.cc: strncmp does not call ENSURE_ASAN_INITED();

[ramosian-glider]

Originally reported on Google Code with ID 94

What steps will reproduce the problem?
1. Write a small program which calls strncmp() as the first thing it does.
2. Ensure that the compiler doesn't optimize out the strncmp() call.
3. Run asanwrapper [executable]

Expected:

* asan should not generate a segfault.

Actual:

* segfault in asan

Note:

strncmp() is missing a call to ENSURE_ASAN_INITED().  Because of this missing call,
when a program attempts to use the asan overloaded strncmp() call, asan segfaults.

The attached patch resolves this problem. This patch is also available from the Android
open source project as https://android-review.googlesource.com/39885

Note 2:

The strcmp() call also looks like it's missing a call to ENSURE_ASAN_INITED(). Please
see https://android-review.googlesource.com/39885

Proposed patch (also attached to this bug as a file)

nnk@anxiety:~/asan/llvm/projects/compiler-rt/lib/asan$ svn diff
Index: asan_interceptors.cc
===================================================================
--- asan_interceptors.cc        (revision 160797)
+++ asan_interceptors.cc        (working copy)
@@ -494,6 +494,7 @@
   if (asan_init_is_running) {
     return REAL(strncmp)(s1, s2, size);
   }
+  ENSURE_ASAN_INITED();
   unsigned char c1 = 0, c2 = 0;
   uptr i;
   for (i = 0; i < size; i++) {

Reported by nnk@google.com on 2012-07-26 19:51:43

---

• Attachment: asan.patch.txt

[ramosian-glider]

fixed in llvm r160853.
eugenis@, do we need to do something special with the Android copy of asan? 

Reported by konstantin.s.serebryany on 2012-07-27 07:13:06

• Status changed: Fixed

[ramosian-glider]

Thanks.
Nick can now update and commit his Android patch.

Reported by eugenis@google.com on 2012-07-27 08:58:07

[ramosian-glider]

I reviewed asan patch 160853, and I think there's a slight bug in it.

In "strcmp", we check the variable "asan_inited". That differs from "strncmp" where
we check the variable "asan_init_is_running". 

The ENSURE_ASAN_INITED call in "strcmp" will never be executed because of the asan_inited
check immediately above.  Or, another way, if you expand the macro in strcmp, you get
the following code where __asan_init() is unreachable.

INTERCEPTOR(int, strcmp, const char *s1, const char *s2) {
  if (!asan_inited) {
    return internal_strcmp(s1, s2);
  }
  do { 
    CHECK(!asan_init_is_running);
    if (!asan_inited) {
      __asan_init();  // UNREACHABLE
    }
  } while (0);
  [deleted]

For reference, here's patch 160852:

$ svn diff -r 160852
Index: asan_interceptors.cc
===================================================================
--- asan_interceptors.cc        (revision 160852)
+++ asan_interceptors.cc        (working copy)
@@ -423,6 +423,7 @@
   if (!asan_inited) {
     return internal_strcmp(s1, s2);
   }
+  ENSURE_ASAN_INITED();
   unsigned char c1, c2;
   uptr i;
   for (i = 0; ; i++) {
@@ -494,6 +495,7 @@
   if (asan_init_is_running) {
     return REAL(strncmp)(s1, s2, size);
   }
+  ENSURE_ASAN_INITED();
   unsigned char c1 = 0, c2 = 0;
   uptr i;
   for (i = 0; i < size; i++) {

Reported by nnk@google.com on 2012-07-27 16:15:56

[ramosian-glider]

>> The ENSURE_ASAN_INITED call in "strcmp" will never be executed because of the asan_inited
check immediately above. 

Well, someone else will eventually call __asan_init. 
So, effectively the call to ENSURE_ASAN_INITED is a no-op, but a harmless one. 
No? 

Alexey, could you please review the current situation? 

Reported by konstantin.s.serebryany on 2012-07-27 16:43:45

• Status changed: New

[ramosian-glider]

Agreed. The code as currently written is a harmless no-op.

Reported by nnk@google.com on 2012-07-27 16:46:25

[ramosian-glider]

I changed
if (!asan_inited) return internal_strcmp(s1, s2);
to
if (asan_init_is_running) return REAL(strcmp)(s1, s2);
for consistency with other interceptors. I think the code should be ok now (r161109)

Reported by samsonov@google.com on 2012-08-01 13:01:22

• Status changed: Fixed

[ramosian-glider]

thanks!

Reported by nnk@google.com on 2012-08-01 20:28:21

[ramosian-glider]

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:59

• Labels added: ProjectAddressSanitizer