[asan] migrate runtime from llvm

Submitted by Wei Mi on Oct. 20, 2012, 1:41 a.m.

Details

Message ID CA+4CFy7jQJTcZ4eD=a-U3e6Kug5QuhMT9B5_6YrQAOx7K6sLHg@mail.gmail.com
State New
Headers show

Commit Message

Wei Mi Oct. 20, 2012, 1:41 a.m.
Thanks David. Here is the patch after removing m4 directory under libasan.
And I add the dependence libasan on libstdc++-v3 to avoid problem
under parallel making for now.

The patch is too big even after compressed, so I have to split the
patch into two parts contained in two mails. patch.part1.txt and
patch.part2.txt. This is the first part.

 // generated by the libgomp configure.  Unfortunately, due to the use of
 //  recursive make, we can't be that specific.

Thanks,
Wei.

On Fri, Oct 19, 2012 at 12:52 PM, Xinliang David Li <davidxl@google.com> wrote:
> The library builds fine with the following diff. The file acinclude.m4
> is cloned from libmudflap.
>
> David
>
> index 485d169..3e847f1 100644
> --- a/libasan/aclocal.m4
> +++ b/libasan/aclocal.m4
> @@ -1037,8 +1037,8 @@ AC_SUBST([am__tar])
>  AC_SUBST([am__untar])
>  ]) # _AM_PROG_TAR
>
> -m4_include([m4/libtool.m4])
> -m4_include([m4/ltoptions.m4])
> -m4_include([m4/ltsugar.m4])
> -m4_include([m4/ltversion.m4])
> -m4_include([m4/lt~obsolete.m4])
> +m4_include([../config/ltoptions.m4])
> +m4_include([../config/ltsugar.m4])
> +m4_include([../config/ltversion.m4])
> +#m4_include([../config/lt~obsolete.m4])
> +m4_include([acinclude.m4])
>
>
> On Fri, Oct 19, 2012 at 11:02 AM, Wei Mi <wmi@google.com> wrote:
>> David, I put the m4 subdir under libasan because once I use the .m4
>> files (libtool.m4  lt~obsolete.m4  ltoptions.m4  ltsugar.m4
>> ltversion.m4) and ltmain.sh under $topsrcdir, the problem that a bad
>> libtool was generated under
>> $topbuilddir/x86_64-unknown-linux-gnu/libasan you met yesterday
>> appeared.  That is why I had to generate the new libtool m4 files and
>> ltmain.sh using libtoolize.
>>
>> Thanks,
>> Wei.
>>
>> On Fri, Oct 19, 2012 at 10:16 AM, Xinliang David Li <davidxl@google.com> wrote:
>>> I tried it, and this version works for me.
>>>
>>> Your probably do not need to add the m4 subdir under libasan.  The
>>> required m4 files are either in .. or ../config dir. See how
>>> libmudflap does it.
>>>
>>> Other than that, if there are no other comments, the change is good to
>>> check into the branch. Remaining bugs can always be found and fixed
>>> later.
>>>
>>> thanks,
>>>
>>> David
>>>
>>>
>>>
>>> On Thu, Oct 18, 2012 at 8:04 PM, Wei Mi <wmi@google.com> wrote:
>>>> Hi,
>>>>
>>>> David cought a problem in the last patch when he tries to built
>>>> libasan. The problem was that an incomplete libtool under libasan
>>>> build directory was generated. The cause is that the old patch used an
>>>> old ltmain.sh to generate libtool. I fix it and attach a new patch.
>>>> And the new patch move -lpthread and -ldl to libasan LDFLAGS.
>>>>
>>>> Thanks,
>>>> Wei.

Comments

Wei Mi Oct. 20, 2012, 1:42 a.m.
And this is the second part.

Thanks,
Wei.

On Fri, Oct 19, 2012 at 6:41 PM, Wei Mi <wmi@google.com> wrote:
> Thanks David. Here is the patch after removing m4 directory under libasan.
> And I add the dependence libasan on libstdc++-v3 to avoid problem
> under parallel making for now.
>
> The patch is too big even after compressed, so I have to split the
> patch into two parts contained in two mails. patch.part1.txt and
> patch.part2.txt. This is the first part.
>
> --- a/Makefile.def      2012-10-19 15:36:36.156106282 -0700
> +++ b/Makefile.def      2012-10-19 15:36:51.656186869 -0700
> @@ -504,7 +504,6 @@
>  dependencies = { module=configure-target-libobjc;
> on=configure-target-boehm-gc; };
>  dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; };
>  dependencies = { module=configure-target-libstdc++-v3;
> on=configure-target-libgomp; };
> +dependencies = { module=configure-target-libasan;
> on=all-target-libstdc++-v3; };
>  // parallel_list.o and parallel_settings.o depend on omp.h, which is
>  // generated by the libgomp configure.  Unfortunately, due to the use of
>  //  recursive make, we can't be that specific.
>
> Thanks,
> Wei.
>
> On Fri, Oct 19, 2012 at 12:52 PM, Xinliang David Li <davidxl@google.com> wrote:
>> The library builds fine with the following diff. The file acinclude.m4
>> is cloned from libmudflap.
>>
>> David
>>
>> index 485d169..3e847f1 100644
>> --- a/libasan/aclocal.m4
>> +++ b/libasan/aclocal.m4
>> @@ -1037,8 +1037,8 @@ AC_SUBST([am__tar])
>>  AC_SUBST([am__untar])
>>  ]) # _AM_PROG_TAR
>>
>> -m4_include([m4/libtool.m4])
>> -m4_include([m4/ltoptions.m4])
>> -m4_include([m4/ltsugar.m4])
>> -m4_include([m4/ltversion.m4])
>> -m4_include([m4/lt~obsolete.m4])
>> +m4_include([../config/ltoptions.m4])
>> +m4_include([../config/ltsugar.m4])
>> +m4_include([../config/ltversion.m4])
>> +#m4_include([../config/lt~obsolete.m4])
>> +m4_include([acinclude.m4])
>>
>>
>> On Fri, Oct 19, 2012 at 11:02 AM, Wei Mi <wmi@google.com> wrote:
>>> David, I put the m4 subdir under libasan because once I use the .m4
>>> files (libtool.m4  lt~obsolete.m4  ltoptions.m4  ltsugar.m4
>>> ltversion.m4) and ltmain.sh under $topsrcdir, the problem that a bad
>>> libtool was generated under
>>> $topbuilddir/x86_64-unknown-linux-gnu/libasan you met yesterday
>>> appeared.  That is why I had to generate the new libtool m4 files and
>>> ltmain.sh using libtoolize.
>>>
>>> Thanks,
>>> Wei.
>>>
>>> On Fri, Oct 19, 2012 at 10:16 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>> I tried it, and this version works for me.
>>>>
>>>> Your probably do not need to add the m4 subdir under libasan.  The
>>>> required m4 files are either in .. or ../config dir. See how
>>>> libmudflap does it.
>>>>
>>>> Other than that, if there are no other comments, the change is good to
>>>> check into the branch. Remaining bugs can always be found and fixed
>>>> later.
>>>>
>>>> thanks,
>>>>
>>>> David
>>>>
>>>>
>>>>
>>>> On Thu, Oct 18, 2012 at 8:04 PM, Wei Mi <wmi@google.com> wrote:
>>>>> Hi,
>>>>>
>>>>> David cought a problem in the last patch when he tries to built
>>>>> libasan. The problem was that an incomplete libtool under libasan
>>>>> build directory was generated. The cause is that the old patch used an
>>>>> old ltmain.sh to generate libtool. I fix it and attach a new patch.
>>>>> And the new patch move -lpthread and -ldl to libasan LDFLAGS.
>>>>>
>>>>> Thanks,
>>>>> Wei.
Xinliang David Li Oct. 25, 2012, 4:24 p.m.
To accommodate tsan (and msan in the future), the directory structure
needs to be changed. The top level directory can be called something
like librt with the following subdirs: asan, tsan, sanitizer_common,
and interception.  Also the libasan should be linked in statically by
default.

thanks,

David

On Fri, Oct 19, 2012 at 6:42 PM, Wei Mi <wmi@google.com> wrote:
> And this is the second part.
>
> Thanks,
> Wei.
>
> On Fri, Oct 19, 2012 at 6:41 PM, Wei Mi <wmi@google.com> wrote:
>> Thanks David. Here is the patch after removing m4 directory under libasan.
>> And I add the dependence libasan on libstdc++-v3 to avoid problem
>> under parallel making for now.
>>
>> The patch is too big even after compressed, so I have to split the
>> patch into two parts contained in two mails. patch.part1.txt and
>> patch.part2.txt. This is the first part.
>>
>> --- a/Makefile.def      2012-10-19 15:36:36.156106282 -0700
>> +++ b/Makefile.def      2012-10-19 15:36:51.656186869 -0700
>> @@ -504,7 +504,6 @@
>>  dependencies = { module=configure-target-libobjc;
>> on=configure-target-boehm-gc; };
>>  dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; };
>>  dependencies = { module=configure-target-libstdc++-v3;
>> on=configure-target-libgomp; };
>> +dependencies = { module=configure-target-libasan;
>> on=all-target-libstdc++-v3; };
>>  // parallel_list.o and parallel_settings.o depend on omp.h, which is
>>  // generated by the libgomp configure.  Unfortunately, due to the use of
>>  //  recursive make, we can't be that specific.
>>
>> Thanks,
>> Wei.
>>
>> On Fri, Oct 19, 2012 at 12:52 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> The library builds fine with the following diff. The file acinclude.m4
>>> is cloned from libmudflap.
>>>
>>> David
>>>
>>> index 485d169..3e847f1 100644
>>> --- a/libasan/aclocal.m4
>>> +++ b/libasan/aclocal.m4
>>> @@ -1037,8 +1037,8 @@ AC_SUBST([am__tar])
>>>  AC_SUBST([am__untar])
>>>  ]) # _AM_PROG_TAR
>>>
>>> -m4_include([m4/libtool.m4])
>>> -m4_include([m4/ltoptions.m4])
>>> -m4_include([m4/ltsugar.m4])
>>> -m4_include([m4/ltversion.m4])
>>> -m4_include([m4/lt~obsolete.m4])
>>> +m4_include([../config/ltoptions.m4])
>>> +m4_include([../config/ltsugar.m4])
>>> +m4_include([../config/ltversion.m4])
>>> +#m4_include([../config/lt~obsolete.m4])
>>> +m4_include([acinclude.m4])
>>>
>>>
>>> On Fri, Oct 19, 2012 at 11:02 AM, Wei Mi <wmi@google.com> wrote:
>>>> David, I put the m4 subdir under libasan because once I use the .m4
>>>> files (libtool.m4  lt~obsolete.m4  ltoptions.m4  ltsugar.m4
>>>> ltversion.m4) and ltmain.sh under $topsrcdir, the problem that a bad
>>>> libtool was generated under
>>>> $topbuilddir/x86_64-unknown-linux-gnu/libasan you met yesterday
>>>> appeared.  That is why I had to generate the new libtool m4 files and
>>>> ltmain.sh using libtoolize.
>>>>
>>>> Thanks,
>>>> Wei.
>>>>
>>>> On Fri, Oct 19, 2012 at 10:16 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>>> I tried it, and this version works for me.
>>>>>
>>>>> Your probably do not need to add the m4 subdir under libasan.  The
>>>>> required m4 files are either in .. or ../config dir. See how
>>>>> libmudflap does it.
>>>>>
>>>>> Other than that, if there are no other comments, the change is good to
>>>>> check into the branch. Remaining bugs can always be found and fixed
>>>>> later.
>>>>>
>>>>> thanks,
>>>>>
>>>>> David
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Oct 18, 2012 at 8:04 PM, Wei Mi <wmi@google.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> David cought a problem in the last patch when he tries to built
>>>>>> libasan. The problem was that an incomplete libtool under libasan
>>>>>> build directory was generated. The cause is that the old patch used an
>>>>>> old ltmain.sh to generate libtool. I fix it and attach a new patch.
>>>>>> And the new patch move -lpthread and -ldl to libasan LDFLAGS.
>>>>>>
>>>>>> Thanks,
>>>>>> Wei.
Jakub Jelinek Oct. 25, 2012, 4:39 p.m.
On Thu, Oct 25, 2012 at 09:24:51AM -0700, Xinliang David Li wrote:
> To accommodate tsan (and msan in the future), the directory structure
> needs to be changed. The top level directory can be called something
> like librt with the following subdirs: asan, tsan, sanitizer_common,
> and interception.  Also the libasan should be linked in statically by
> default.

librt is a very bad name, that clashes with glibc librt, would only create
confusion.

Why should be libasan linked statically by default?

	Jakub
Xinliang David Li Oct. 25, 2012, 4:48 p.m.
On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 25, 2012 at 09:24:51AM -0700, Xinliang David Li wrote:
>> To accommodate tsan (and msan in the future), the directory structure
>> needs to be changed. The top level directory can be called something
>> like librt with the following subdirs: asan, tsan, sanitizer_common,
>> and interception.  Also the libasan should be linked in statically by
>> default.
>
> librt is a very bad name, that clashes with glibc librt, would only create
> confusion.

Ok, then we should pick something that is not confusing but reflect
the fact  they are for runtime error checking ..

>
> Why should be libasan linked statically by default?
>

There are a couple of reasons:

1) it makes running sanitized binary on remote machines which does not
have libasan installed easier;
2) There is no guarantee that libasan API won't change, statically
linking it in makes it less vulnerable to such changes.

thanks,

David

>         Jakub
Diego Novillo Oct. 25, 2012, 4:52 p.m.
On Thu, Oct 25, 2012 at 12:48 PM, Xinliang David Li <davidxl@google.com> wrote:
> On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> librt is a very bad name, that clashes with glibc librt, would only create
>> confusion.
>
> Ok, then we should pick something that is not confusing but reflect
> the fact  they are for runtime error checking ..

libsanitizer?


Diego.
Xinliang David Li Oct. 25, 2012, 4:54 p.m.
On Thu, Oct 25, 2012 at 9:52 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Thu, Oct 25, 2012 at 12:48 PM, Xinliang David Li <davidxl@google.com> wrote:
>> On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>
>>> librt is a very bad name, that clashes with glibc librt, would only create
>>> confusion.
>>
>> Ok, then we should pick something that is not confusing but reflect
>> the fact  they are for runtime error checking ..
>
> libsanitizer?

+1 --- missed the obvious one :)

David

>
>
> Diego.
Jakub Jelinek Oct. 25, 2012, 4:55 p.m.
On Thu, Oct 25, 2012 at 09:48:54AM -0700, Xinliang David Li wrote:
> > Why should be libasan linked statically by default?

> There are a couple of reasons:
> 
> 1) it makes running sanitized binary on remote machines which does not
> have libasan installed easier;
> 2) There is no guarantee that libasan API won't change, statically
> linking it in makes it less vulnerable to such changes.

Neither of the reasons look important enough, compared to the
http://www.akkadia.org/drepper/no_static_linking.html
Especially as libasan is overriding malloc etc., having dozens of malloc
overrides in different shared libraries, perhaps all chaining into each
other, is a nightmare.
For 2), libasan should just use symbol versioning and provide backwards
compatibility as long as it is possible, or worst case bump SONAME
occassionally.

	Jakub
Xinliang David Li Oct. 25, 2012, 5 p.m.
On Thu, Oct 25, 2012 at 9:55 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 25, 2012 at 09:48:54AM -0700, Xinliang David Li wrote:
>> > Why should be libasan linked statically by default?
>
>> There are a couple of reasons:
>>
>> 1) it makes running sanitized binary on remote machines which does not
>> have libasan installed easier;
>> 2) There is no guarantee that libasan API won't change, statically
>> linking it in makes it less vulnerable to such changes.
>
> Neither of the reasons look important enough, compared to the
> http://www.akkadia.org/drepper/no_static_linking.html
> Especially as libasan is overriding malloc etc., having dozens of malloc
> overrides in different shared libraries, perhaps all chaining into each
> other, is a nightmare.


How about statically linking just for executables, not shared library buid?

David

> For 2), libasan should just use symbol versioning and provide backwards
> compatibility as long as it is possible, or worst case bump SONAME
> occassionally.
>
>         Jakub
Jakub Jelinek Oct. 25, 2012, 5:19 p.m.
On Thu, Oct 25, 2012 at 10:00:03AM -0700, Xinliang David Li wrote:
> How about statically linking just for executables, not shared library buid?

That is IMHO still a bad idea.

	Jakub
Xinliang David Li Oct. 25, 2012, 5:24 p.m.
On Thu, Oct 25, 2012 at 10:19 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 25, 2012 at 10:00:03AM -0700, Xinliang David Li wrote:
>> How about statically linking just for executables, not shared library buid?
>
> That is IMHO still a bad idea.

I don't know why you think so (It seems that the points mentioned in
http://www.akkadia.org/drepper/no_static_linking.html mainly apply to
release binaries, not sanitized ones), but for now let's drop the
static link request.

thanks,

David


>
>         Jakub
Diego Novillo Oct. 25, 2012, 5:27 p.m.
On Thu, Oct 25, 2012 at 1:24 PM, Xinliang David Li <davidxl@google.com> wrote:

> I don't know why you think so (It seems that the points mentioned in
> http://www.akkadia.org/drepper/no_static_linking.html mainly apply to
> release binaries, not sanitized ones), but for now let's drop the
> static link request.

Yeah, we can always just do that in our own builds.


Diego.

Patch hide | download patch | download mbox

--- a/Makefile.def      2012-10-19 15:36:36.156106282 -0700
+++ b/Makefile.def      2012-10-19 15:36:51.656186869 -0700
@@ -504,7 +504,6 @@ 
 dependencies = { module=configure-target-libobjc;
on=configure-target-boehm-gc; };
 dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; };
 dependencies = { module=configure-target-libstdc++-v3;
on=configure-target-libgomp; };
+dependencies = { module=configure-target-libasan;
on=all-target-libstdc++-v3; };
 // parallel_list.o and parallel_settings.o depend on omp.h, which is