[m-rev.] For review: Workaround Linux NTPL TSX bug (Mercury Bug: 334)

[Julien Fischer]

On Tue, 1 Jul 2014, Julien Fischer wrote:

>
>
> On Fri, 27 Jun 2014, Paul Bone wrote:
>
>> On Fri, Jun 27, 2014 at 05:16:09PM +1000, Peter Wang wrote:
>>> On Fri, 27 Jun 2014 16:18:26 +1000, Paul Bone <paul at bone.id.au> wrote:
>>>> On Fri, Jun 27, 2014 at 03:33:46PM +1000, Peter Wang wrote:
>>>>> On Fri, 27 Jun 2014 11:27:40 +1000, Paul Bone <paul at bone.id.au> wrote:
>>>>>> 
>>>>>> Of course my change here is only a workaround.  The real fix is 
>>>>>> patching
>>>>>> glibc.
>>>>> 
>>>>> Well, I prefer you do not commit it.  If it's just to continue your
>>>>> development, I think you could rebuild glibc with --disable-lock-elision
>>>>> and LD_PRELOAD libpthread?
>>>> 
>>>> What if other users have the same error?  This affects all
>>>> asm_fast.gc.par.stseg grades.  I can't resonably ask other users, 
>>>> including
>>>> yourself, either to buy a processor without this extension or to always 
>>>> use
>>>> an older version of glibc.
>>> 
>>> I guess I'm expecting that it to be fixed one way or another, in due
>>> time, if more popular Boehm GC-using software is affected.
>> 
>> I've no intention of waiting the unspecified amount of time it will take 
>> for
>>    1) the bug to be acknowledged*
>>    2) the bug to be patched
>>    3) the patches to trickle down to actually be installed on people's
>>       systems.
>>
>>  *: I don't think that the developers of glibc and/or Andi will actually be
>>     any worse than any other developer in this regard.  I'm not making a
>>     personal attack I'm just cynical.
>
> This change has broken compilation of the collector on OS X (and
> probably other systems) (in hlc.par.gc)
>
> cp gc.a libpar_gc.a
> clang -multiply_defined suppress                      -dynamiclib 
> -single_module -install_name \
> 		/Users/jfischer/Mercury-Install/mercury-14.01.1-beta-2014-07-01/bin/lib/mercury/lib/libpar_gc.dylib 
> \
> 		-o libpar_gc.dylib alloc.o reclaim.o allchblk.o misc.o 
> mach_dep.o os_dep.o mark_rts.o headers.o mark.o obj_map.o blacklst.o 
> finalize.o new_hblk.o dbg_mlc.o malloc.o stubborn.o checksums.o 
> pthread_support.o pthread_stop_world.o darwin_stop_world.o typd_mlc.o 
> ptr_chck.o mallocx.o gcj_mlc.o specific.o gc_dlopen.o backgraph.o 
> win32_threads.o pthread_start.o thread_local_alloc.o dyn_load.o -lc Undefined 
> symbols for architecture x86_64:
>  "_GC_setup_mark_lock", referenced from:
>      _GC_init in misc.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see 
> invocation)
> make[2]: *** [libpar_gc.dylib] Error 1
> make[1]: *** [submake] Error 2
> To clean up from failed install, remove 
> /Users/jfischer/Mercury-Workspaces/mercury.local-4/stage2/install_grade_dir.hlc.par.gc
> make: *** [install_grades] Error 1
>
> Please do not modify the Boehm GC unless you have first tested your
> changes on multiple OSs, not just Linux.  The code in the collector
> related to threads is far too delicate to just assume that it is going
> to work.

In particular, you probably want to restrict your workaround to Linux,
as in the following patch.  (I don't think GC_PTHREADS means what you
think it means.)

diff --git a/boehm_gc/misc.c b/boehm_gc/misc.c
index 1b65243..86457b4 100644
--- a/boehm_gc/misc.c
+++ b/boehm_gc/misc.c
@@ -744,9 +744,9 @@ GC_API void GC_CALL GC_init(void)
          /* else */ InitializeCriticalSection (&GC_allocate_ml);
       }
  #   endif /* GC_WIN32_THREADS */
-#   if (defined(GC_PTHREADS) && !defined(GC_WIN32_THREADS))
+#   if (defined(GC_LINUX_THREADS))
       GC_setup_mark_lock();
-#   endif /* GC_PTHREADS */
+#   endif /* GC_LINUX_THREADS */
  #   if (defined(MSWIN32) || defined(MSWINCE)) && defined(THREADS)
        InitializeCriticalSection(&GC_write_cs);
  #   endif

Cheers,
Julien.