Bug 78307 - [7 Regression] missing symbols in libubsan without changing the soname
Summary: [7 Regression] missing symbols in libubsan without changing the soname
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 7.0
: P1 normal
Target Milestone: 7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ABI
Depends on:
Blocks:
 
Reported: 2016-11-11 09:33 UTC by Matthias Klose
Modified: 2016-11-18 14:32 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 6.2.1
Known to fail: 7.0
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2016-11-11 09:33:09 UTC
on trunk 20161110, libubsan is missing some symbols while not changing the soname:

libubsan:
+#MISSING: 7-20161110-1# __ubsan_handle_cfi_bad_icall@Base 6
+#MISSING: 7-20161110-1# __ubsan_handle_cfi_bad_icall_abort@Base 6
+#MISSING: 7-20161110-1# __ubsan_handle_cfi_bad_type@Base 6
+#MISSING: 7-20161110-1# __ubsan_handle_cfi_bad_type_abort@Base 6
Comment 1 Maxim Ostapenko 2016-11-11 13:23:54 UTC
Did this cause the link error?

AFAIK GCC doesn't support CFI thus doesn't emit these symbols at compiler side.
Comment 2 Matthias Klose 2016-11-11 13:25:46 UTC
I can't tell. I was just looking at symbol difference in the library, like using the abigail tools.
Comment 3 Jakub Jelinek 2016-11-11 13:27:11 UTC
Sure, it doesn't affect gcc emitted code unless somebody calls those directly, but perhaps say llvm compiled code using the shared libubsan.so.  In any case, we shouldn't be making ABI incompatible changes in the libraries.  So, either we should bump soname, or preferably, if it is not that hard to readd those symbols, just do that, so that people don't have to fight yet another changed library.
Comment 4 Maxim Ostapenko 2016-11-11 13:37:45 UTC
(In reply to Jakub Jelinek from comment #3)
> Sure, it doesn't affect gcc emitted code unless somebody calls those
> directly, but perhaps say llvm compiled code using the shared libubsan.so. 

But LLVM doesn't support shared UBSan runtime (the only one supported is ASan) and AFAIK there aren't any plans to support it there.

> In any case, we shouldn't be making ABI incompatible changes in the
> libraries.  So, either we should bump soname, or preferably, if it is not
> that hard to readd those symbols, just do that, so that people don't have to
> fight yet another changed library.

Do you mean we can apply a local patch?
Comment 5 Jakub Jelinek 2016-11-11 13:40:14 UTC
(In reply to Maxim Ostapenko from comment #4)
> But LLVM doesn't support shared UBSan runtime (the only one supported is
> ASan) and AFAIK there aren't any plans to support it there.

Yeah, it is a very weird policy.

> > In any case, we shouldn't be making ABI incompatible changes in the
> > libraries.  So, either we should bump soname, or preferably, if it is not
> > that hard to readd those symbols, just do that, so that people don't have to
> > fight yet another changed library.
> 
> Do you mean we can apply a local patch?

We can, sure.
Comment 6 Maxim Ostapenko 2016-11-11 13:45:49 UTC
(In reply to Jakub Jelinek from comment #5)
> (In reply to Maxim Ostapenko from comment #4)
> > But LLVM doesn't support shared UBSan runtime (the only one supported is
> > ASan) and AFAIK there aren't any plans to support it there.
> 
> Yeah, it is a very weird policy.
> 
> > > In any case, we shouldn't be making ABI incompatible changes in the
> > > libraries.  So, either we should bump soname, or preferably, if it is not
> > > that hard to readd those symbols, just do that, so that people don't have to
> > > fight yet another changed library.
> > 
> > Do you mean we can apply a local patch?
> 
> We can, sure.

Ok, I think I can just add (perhaps empty?) stubs into libubsan to readd those symbols, this should be quite trivial.
Comment 7 Jakub Jelinek 2016-11-11 13:59:56 UTC
(In reply to Maxim Ostapenko from comment #6)
> (In reply to Jakub Jelinek from comment #5)
> > (In reply to Maxim Ostapenko from comment #4)
> > > But LLVM doesn't support shared UBSan runtime (the only one supported is
> > > ASan) and AFAIK there aren't any plans to support it there.
> > 
> > Yeah, it is a very weird policy.
> > 
> > > > In any case, we shouldn't be making ABI incompatible changes in the
> > > > libraries.  So, either we should bump soname, or preferably, if it is not
> > > > that hard to readd those symbols, just do that, so that people don't have to
> > > > fight yet another changed library.
> > > 
> > > Do you mean we can apply a local patch?
> > 
> > We can, sure.
> 
> Ok, I think I can just add (perhaps empty?) stubs into libubsan to readd
> those symbols, this should be quite trivial.

I think we shouldn't add empty functions, instead look at upstream
http://llvm.org/viewvc/llvm-project?view=revision&revision=258744
change and undo the - parts of it essentially - most likely translate the old
CFIBadTypeData structure we are given into the new CFICheckFailData and just call HandleCFIBadType.
Comment 8 chefmax 2016-11-16 11:13:51 UTC
Author: chefmax
Date: Wed Nov 16 11:13:19 2016
New Revision: 242478

URL: https://gcc.gnu.org/viewcvs?rev=242478&root=gcc&view=rev
Log:
	PR sanitizer/78307
	* ubsan/ubsan_handlers.cc (__ubsan_handle_cfi_bad_icall): New function.
	( __ubsan_handle_cfi_bad_icall_abort): Likewise. 
	* ubsan/ubsan_handlers.h (struct CFIBadIcallData): New type.
	* ubsan/ubsan_handlers_cxx.cc (__ubsan_handle_cfi_bad_type): New
	function.
	(__ubsan_handle_cfi_bad_type_abort): Likewise.
	* ubsan/ubsan_handlers_cxx.h (struct CFIBadTypeData): New type.
	(__ubsan_handle_cfi_bad_type): Export function.
	(__ubsan_handle_cfi_bad_type_abort): Likewise.
	* HOWTO_MERGE: Update documentation.

Modified:
    trunk/libsanitizer/ChangeLog
    trunk/libsanitizer/HOWTO_MERGE
    trunk/libsanitizer/ubsan/ubsan_handlers.cc
    trunk/libsanitizer/ubsan/ubsan_handlers.h
    trunk/libsanitizer/ubsan/ubsan_handlers_cxx.cc
    trunk/libsanitizer/ubsan/ubsan_handlers_cxx.h
Comment 9 Maxim Ostapenko 2016-11-18 14:31:20 UTC
Can we close this?
Comment 10 Jakub Jelinek 2016-11-18 14:32:44 UTC
I think so.