Avro
  1. Avro
  2. AVRO-1033

Avro-C reference count fails on x86 and x86-64 for GCC 4.1.0 and earlier

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.2
    • Fix Version/s: 1.6.3
    • Component/s: c
    • Labels:
      None
    • Environment:

      Linux on an x86 or x86-64 platform with GCC version less than or equal to 4.1.0.

      Description

      Item 1:
      *******

      In the Avro-C implementation, there is a bug in the Raw x86 assembly
      performing the atomic increment and decrement operations in
      refcount.h. Specifically, the assembly language commands (mistakenly)
      operate on the the pointer (refcount) instead of operating on the
      memory pointed to (*refcount). This results in memory leaks, when
      trying to deallocate avro values.

      This problem only occurs when using GCC versions less than 4.1.0. For
      GCC versions greater than 4.1.0, the code uses the GCC intrinsic
      __sync_add_and_fetch() or __sync_sub_and_fetch() instead of the raw
      x86 assembly, and this works properly.

      Since this problem is masked for developers working with new(er) GCC
      compilers, greater than version 4.1.0, one way to "uncover" it is to
      force the GCC intrinsics off by requiring a higher GCC version (say
      8.1.0) to use the GCC intrinsics. Then GCC falls back to using the x86
      assembly, and the problem can be seen.

      I am attaching a sample test case (test_refcount.c), which will show a
      memory leak when compiled with the raw x86 commands enabled in
      refcount.h and run with valgrind using the following command:
      valgrind -v --leak-check=full --track-origins=yes ./test_refcount

      I am also attaching a simple patch to fix the bug. The patch just
      replaces (refcount) with (*refcount) in a few places, and the memory
      leak disappears.

      Item 2:
      *******

      On a related note, while compiling Avro across several flavors of
      Linux, we noticed that the GCC atomic intrinsics
      __sync_add_and_fetch() and __sync_sub_and_fetch() were not implemented
      for all versions of GCC greater than 4.1.0, causing compile errors.

      Specifically, the following two versions of GCC don't appear to
      support the GCC atomic intrinsics:
      1. GCC 4.1.2 20080704 (Red Hat 4.1.2-48)
      2. GCC 4.4.3 on Mandriva for an i586 target

      In our local code we have updated the GCC version test in refcount.h
      to require versions of GCC greater than 4.5.0, before including the
      GCC intrinsics. We might want to do the same in the global Avro
      repository. If appropriate, I can create a separate JIRA item for
      this issue. Please let me know.

      Cheers,
      Vivek Nadkarni

      1. AVRO-1033.patch
        0.7 kB
        Vivek Nadkarni
      2. test_refcount.c
        1 kB
        Vivek Nadkarni

        Activity

        Doug Cutting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Douglas Creager made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Assignee Douglas Creager [ dcreager ]
        Resolution Fixed [ 1 ]
        Vivek Nadkarni made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Vivek Nadkarni made changes -
        Field Original Value New Value
        Attachment AVRO-1033.patch [ 12515198 ]
        Attachment test_refcount.c [ 12515199 ]
        Vivek Nadkarni created issue -

          People

          • Assignee:
            Douglas Creager
            Reporter:
            Vivek Nadkarni
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development