[HP aCC 6.16] Out of bound access in new.cpp

• Key: STDCXX-761
• Type: Sub-task
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Eric Lemings
• Reporter: Scott (Yu) Zhong
• Component/s: Test Driver
• Affects Version/s: 4.2.0
• Fix Version/s: 4.2.1

Environment

$ uname -sr && aCC -V
HP-UX B.11.31
aCC: HP C/aC++ B3910B A.06.16 [Nov 26 2007]

Description

When compiled with HP aCC 6.16 +w +O, the test driver source file new.cpp produces the warnings below:

aCC -c     -I$(TOPDIR)/include -I$(BUILDDIR)/include -I$(TOPDIR)/tests/include -AA  +O2  +DD64 +w \
    +W392 +W655 +W684 +W818 +W819 +W849 +W2193 +W2236 +W2261 +W2340 +W2401 +W2487 +W4227 \
    +W4229 +W4231 +W4235 +W4237 +W4249 +W4255 +W4272 +W4284 +W4285 +W4286 +W4296 +W4297 +W3348 \
    $(TOPDIR)/tests/src/new.cpp
"$(TOPDIR)/tests/src/new.cpp", line 629, procedure rwt_checkpoint: warning #20206-D: Out of bound access (In expression "(unsigned long long*)(&diff)->new_calls_+i", (&diff)->new_calls_ (type: unsigned long long [2]) has byte range [0 .. 15], writing byte range [0 .. 127].)

Comments

[Martin Sebor]

Added context, including command line, and removed formatting.

Brad, you might want to look into this while investigating STDCXX-709.

[Scott (Yu) Zhong]

add compile line to description

[Eric Lemings]

According to the definition of rwt_free_store in tests/include/rw_new.h, the array member clearly has an extent of 2. Why the loop in file tests/src/new.cpp at line 629 is hard-coded for 16 iterations is bewildering. Unless someone knows why this is, I'm inclined to just change it to 2.

[Travis Vitek]

It looks like someone is being tricky. They are assuming that every member in rwt_free_store is contiguous [i.e. no padding], and they are actually comparing each of new_calls_, delete_calls_, blocks_, bytes_, max_blocks_, max_bytes_ and max_block_size_ with that one loop. If you decide to fix this, you'll probably end up writing eight loops of two iterations each.

Also, I'd prefer that the 2 isn't hardcoded and that you use something like sizeof (array) / sizeof (*array), or a size macro instead.

[Eric Lemings]

Travis,

If you could, give the following patch a whirl (or quick review
at least).

$ svn diff
Index: tests/src/new.cpp
===================================================================
--- tests/src/new.cpp   (revision 644444)
+++ tests/src/new.cpp   (working copy)
@@ -604,6 +604,17 @@
     return ret;
 }

+static void
+rwt_checkpoint_compare (_RWSTD_SIZE_T* diffs, _RWSTD_SIZE_T n,
+                        const _RWSTD_SIZE_T* st0, const _RWSTD_SIZE_T*
st1,
+                        bool& diff_0)
+{
+    for (_RWSTD_SIZE_T i = 0; i != n; ++i) {
+        diffs [i] = st1 [i] - st0 [i];
+        if (diffs [i])
+            diff_0 = false;
+    }
+}

 _TEST_EXPORT rwt_free_store*
 rwt_checkpoint (const rwt_free_store *st0, const rwt_free_store *st1)
@@ -616,21 +627,24 @@
         // of the free_store specified by the arguments

         static rwt_free_store diff;
-
         memset (&diff, 0, sizeof diff);

-        size_t*       diffs    = diff.new_calls_;
-        const size_t* st0_args = st0->new_calls_;
-        const size_t* st1_args = st1->new_calls_;
-
         bool diff_0 = true;   // difference of 0 (i.e., none)

-        for (size_t i = 0; i != 16; ++i) {
-            diffs [i] = st1_args [i] - st0_args [i];
-            if (diffs [i])
-                diff_0 = false;
-        }
+#define EXTENT(array) (sizeof (array) / sizeof(*array))
+#define COMPARE(member) \
+        rwt_checkpoint_compare (diff.member, EXTENT(diff.member),\
+                                st0->member, st1->member, diff_0)

+        COMPARE(new_calls_);
+        COMPARE(delete_calls_);
+        COMPARE(delete_0_calls_);
+        COMPARE(blocks_);
+        COMPARE(bytes_);
+        COMPARE(max_blocks_);
+        COMPARE(max_bytes_);
+        COMPARE(max_block_size_);
+
         if (diff_0)
             return 0;

The 0.new test seems to be okay with it.

Thanks,
Brad.

[Martin Sebor]

See this thread for a discussion of the patch.

[Eric Lemings]

http://svn.apache.org/viewvc?view=rev&revision=644846

[Farid Zaripov]

Merged in 4.2.x branch thus: http://svn.apache.org/viewvc?view=rev&revision=648752