* [PATCH v2 for-4.10] ubsan: add clang 5.0 support
@ 2017-10-18 7:45 Roger Pau Monne
2017-10-18 9:23 ` Jan Beulich
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Roger Pau Monne @ 2017-10-18 7:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich, Roger Pau Monne
clang 5.0 changed the layout of the type_mismatch_data structure and
introduced __ubsan_handle_type_mismatch_v1 and
__ubsan_handle_pointer_overflow.
This commit adds support for the new structure layout, adds the
missing handlers and the new types for type_check_kinds.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
---
ubsan is an optional feature, not enabled by default and not designed
to be used by production systems. Since this change only touches ubsan
code and it's a bugfix in order for clang to work, I argue it should
be merged into 4.10.
---
Changes since v1:
- Replace message in __ubsan_handle_pointer_overflow.
- Add a suppress_report check in __ubsan_handle_type_mismatch_v1.
---
xen/common/ubsan/ubsan.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
xen/common/ubsan/ubsan.h | 11 +++++++++++
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
index fbe568562a..2eaa403691 100644
--- a/xen/common/ubsan/ubsan.c
+++ b/xen/common/ubsan/ubsan.c
@@ -33,7 +33,10 @@ const char *type_check_kinds[] = {
"member call on",
"constructor call on",
"downcast of",
- "downcast of"
+ "downcast of",
+ "upcast of",
+ "cast to virtual base of",
+ "_Nonnull binding to",
};
#define REPORTED_BIT 31
@@ -333,6 +336,26 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data,
}
EXPORT_SYMBOL(__ubsan_handle_type_mismatch);
+void __ubsan_handle_type_mismatch_v1(struct type_mismatch_data_v1 *data,
+ unsigned long ptr)
+{
+ struct type_mismatch_data d = {
+ .location = data->location,
+ .type = data->type,
+ .alignment = 1ul << data->log_alignment,
+ .type_check_kind = data->type_check_kind,
+ };
+
+ /*
+ * NB: do the check with data->location, d->location is just a local
+ * copy and the modifications will be lost.
+ */
+ if (suppress_report(&data->location))
+ return;
+
+ __ubsan_handle_type_mismatch(&d, ptr);
+}
+
void __ubsan_handle_nonnull_arg(struct nonnull_arg_data *data)
{
unsigned long flags;
@@ -478,3 +501,24 @@ void __ubsan_handle_load_invalid_value(struct invalid_value_data *data,
ubsan_epilogue(&flags);
}
EXPORT_SYMBOL(__ubsan_handle_load_invalid_value);
+
+void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
+ unsigned long base, unsigned long result)
+{
+ unsigned long flags;
+
+ if (suppress_report(&data->location))
+ return;
+
+ ubsan_prologue(&data->location, &flags);
+
+ if (((long)base >= 0) == ((long)result >= 0))
+ pr_err("pointer operation %s %p to %p\n",
+ base > result ? "underflowed" : "overflowed",
+ (void *)base, (void *)result);
+ else
+ pr_err("pointer index expression with base %p overflowed to %p\n",
+ (void *)base, (void *)result);
+
+ ubsan_epilogue(&flags);
+}
diff --git a/xen/common/ubsan/ubsan.h b/xen/common/ubsan/ubsan.h
index b2d18d4a53..2710cd423e 100644
--- a/xen/common/ubsan/ubsan.h
+++ b/xen/common/ubsan/ubsan.h
@@ -36,6 +36,13 @@ struct type_mismatch_data {
unsigned char type_check_kind;
};
+struct type_mismatch_data_v1 {
+ struct source_location location;
+ struct type_descriptor *type;
+ unsigned char log_alignment;
+ unsigned char type_check_kind;
+};
+
struct nonnull_arg_data {
struct source_location location;
struct source_location attr_location;
@@ -73,6 +80,10 @@ struct invalid_value_data {
struct type_descriptor *type;
};
+struct pointer_overflow_data {
+ struct source_location location;
+};
+
#if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
typedef __int128 s_max;
typedef unsigned __int128 u_max;
--
2.13.5 (Apple Git-94)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 7:45 [PATCH v2 for-4.10] ubsan: add clang 5.0 support Roger Pau Monne
@ 2017-10-18 9:23 ` Jan Beulich
2017-10-18 9:42 ` Roger Pau Monné
2017-10-18 10:17 ` Wei Liu
2017-10-18 13:35 ` Julien Grall
2 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-10-18 9:23 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
Andrew Cooper, IanJackson, Tim Deegan, Julien Grall, xen-devel
>>> On 18.10.17 at 09:45, <roger.pau@citrix.com> wrote:
> +void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
> + unsigned long base, unsigned long result)
> +{
> + unsigned long flags;
> +
> + if (suppress_report(&data->location))
> + return;
> +
> + ubsan_prologue(&data->location, &flags);
> +
> + if (((long)base >= 0) == ((long)result >= 0))
> + pr_err("pointer operation %s %p to %p\n",
> + base > result ? "underflowed" : "overflowed",
> + (void *)base, (void *)result);
> + else
> + pr_err("pointer index expression with base %p overflowed to %p\n",
> + (void *)base, (void *)result);
Would you mind explaining the difference between if and else
branches? (I do realize I should have asked this on v1 already,
but I didn't pay enough attention.) Whatever the idea behind
this, it should probably be explained in a comment, as it looks
to be heuristic.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 9:23 ` Jan Beulich
@ 2017-10-18 9:42 ` Roger Pau Monné
2017-10-18 9:53 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: Roger Pau Monné @ 2017-10-18 9:42 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
Andrew Cooper, IanJackson, Tim Deegan, Julien Grall, xen-devel
On Wed, Oct 18, 2017 at 03:23:20AM -0600, Jan Beulich wrote:
> >>> On 18.10.17 at 09:45, <roger.pau@citrix.com> wrote:
> > +void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
> > + unsigned long base, unsigned long result)
> > +{
> > + unsigned long flags;
> > +
> > + if (suppress_report(&data->location))
> > + return;
> > +
> > + ubsan_prologue(&data->location, &flags);
> > +
> > + if (((long)base >= 0) == ((long)result >= 0))
> > + pr_err("pointer operation %s %p to %p\n",
> > + base > result ? "underflowed" : "overflowed",
> > + (void *)base, (void *)result);
> > + else
> > + pr_err("pointer index expression with base %p overflowed to %p\n",
> > + (void *)base, (void *)result);
>
> Would you mind explaining the difference between if and else
> branches? (I do realize I should have asked this on v1 already,
> but I didn't pay enough attention.) Whatever the idea behind
> this, it should probably be explained in a comment, as it looks
> to be heuristic.
The upstream commit is:
https://github.com/llvm-mirror/compiler-rt/commit/079b7657767dcc0fb284225c277d2b9ce73e423b
However it's lacking a proper commit message. It seems to me like it's
there to detect addition of signed + unsigned values when an overflow
happens, but I don't really see it's value rather than just using the
first message.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 9:42 ` Roger Pau Monné
@ 2017-10-18 9:53 ` Jan Beulich
2017-10-18 9:58 ` Roger Pau Monné
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-10-18 9:53 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
Andrew Cooper, IanJackson, Tim Deegan, Julien Grall, xen-devel
>>> On 18.10.17 at 11:42, <roger.pau@citrix.com> wrote:
> On Wed, Oct 18, 2017 at 03:23:20AM -0600, Jan Beulich wrote:
>> >>> On 18.10.17 at 09:45, <roger.pau@citrix.com> wrote:
>> > +void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
>> > + unsigned long base, unsigned long result)
>> > +{
>> > + unsigned long flags;
>> > +
>> > + if (suppress_report(&data->location))
>> > + return;
>> > +
>> > + ubsan_prologue(&data->location, &flags);
>> > +
>> > + if (((long)base >= 0) == ((long)result >= 0))
>> > + pr_err("pointer operation %s %p to %p\n",
>> > + base > result ? "underflowed" : "overflowed",
>> > + (void *)base, (void *)result);
>> > + else
>> > + pr_err("pointer index expression with base %p overflowed to %p\n",
>> > + (void *)base, (void *)result);
>>
>> Would you mind explaining the difference between if and else
>> branches? (I do realize I should have asked this on v1 already,
>> but I didn't pay enough attention.) Whatever the idea behind
>> this, it should probably be explained in a comment, as it looks
>> to be heuristic.
>
> The upstream commit is:
>
> https://github.com/llvm-mirror/compiler-rt/commit/079b7657767dcc0fb284225c277d
> 2b9ce73e423b
>
> However it's lacking a proper commit message. It seems to me like it's
> there to detect addition of signed + unsigned values when an overflow
> happens, but I don't really see it's value rather than just using the
> first message.
Right - me too. I'd therefore like to simply drop the "if" and the "else"
branch (likely easily done while committing), and then the change is
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 9:53 ` Jan Beulich
@ 2017-10-18 9:58 ` Roger Pau Monné
0 siblings, 0 replies; 7+ messages in thread
From: Roger Pau Monné @ 2017-10-18 9:58 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
Andrew Cooper, IanJackson, Tim Deegan, Julien Grall, xen-devel
On Wed, Oct 18, 2017 at 03:53:37AM -0600, Jan Beulich wrote:
> >>> On 18.10.17 at 11:42, <roger.pau@citrix.com> wrote:
> > On Wed, Oct 18, 2017 at 03:23:20AM -0600, Jan Beulich wrote:
> >> >>> On 18.10.17 at 09:45, <roger.pau@citrix.com> wrote:
> >> > +void __ubsan_handle_pointer_overflow(struct pointer_overflow_data *data,
> >> > + unsigned long base, unsigned long result)
> >> > +{
> >> > + unsigned long flags;
> >> > +
> >> > + if (suppress_report(&data->location))
> >> > + return;
> >> > +
> >> > + ubsan_prologue(&data->location, &flags);
> >> > +
> >> > + if (((long)base >= 0) == ((long)result >= 0))
> >> > + pr_err("pointer operation %s %p to %p\n",
> >> > + base > result ? "underflowed" : "overflowed",
> >> > + (void *)base, (void *)result);
> >> > + else
> >> > + pr_err("pointer index expression with base %p overflowed to %p\n",
> >> > + (void *)base, (void *)result);
> >>
> >> Would you mind explaining the difference between if and else
> >> branches? (I do realize I should have asked this on v1 already,
> >> but I didn't pay enough attention.) Whatever the idea behind
> >> this, it should probably be explained in a comment, as it looks
> >> to be heuristic.
> >
> > The upstream commit is:
> >
> > https://github.com/llvm-mirror/compiler-rt/commit/079b7657767dcc0fb284225c277d
> > 2b9ce73e423b
> >
> > However it's lacking a proper commit message. It seems to me like it's
> > there to detect addition of signed + unsigned values when an overflow
> > happens, but I don't really see it's value rather than just using the
> > first message.
>
> Right - me too. I'd therefore like to simply drop the "if" and the "else"
> branch (likely easily done while committing), and then the change is
> Acked-by: Jan Beulich <jbeulich@suse.com>
Yes, feel free to drop the if/else and just keep the first error
message.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 7:45 [PATCH v2 for-4.10] ubsan: add clang 5.0 support Roger Pau Monne
2017-10-18 9:23 ` Jan Beulich
@ 2017-10-18 10:17 ` Wei Liu
2017-10-18 13:35 ` Julien Grall
2 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2017-10-18 10:17 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich, xen-devel
On Wed, Oct 18, 2017 at 08:45:32AM +0100, Roger Pau Monne wrote:
> clang 5.0 changed the layout of the type_mismatch_data structure and
> introduced __ubsan_handle_type_mismatch_v1 and
> __ubsan_handle_pointer_overflow.
>
> This commit adds support for the new structure layout, adds the
> missing handlers and the new types for type_check_kinds.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
With existing comments addressed:
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 for-4.10] ubsan: add clang 5.0 support
2017-10-18 7:45 [PATCH v2 for-4.10] ubsan: add clang 5.0 support Roger Pau Monne
2017-10-18 9:23 ` Jan Beulich
2017-10-18 10:17 ` Wei Liu
@ 2017-10-18 13:35 ` Julien Grall
2 siblings, 0 replies; 7+ messages in thread
From: Julien Grall @ 2017-10-18 13:35 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich
Hi Roger,
On 10/18/2017 08:45 AM, Roger Pau Monne wrote:
> clang 5.0 changed the layout of the type_mismatch_data structure and
> introduced __ubsan_handle_type_mismatch_v1 and
> __ubsan_handle_pointer_overflow.
>
> This commit adds support for the new structure layout, adds the
> missing handlers and the new types for type_check_kinds.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> ubsan is an optional feature, not enabled by default and not designed
> to be used by production systems. Since this change only touches ubsan
> code and it's a bugfix in order for clang to work, I argue it should
> be merged into 4.10.
I agree here:
Release-acked-by: Julien Grall <julien.grall@linaro.org>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-18 13:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18 7:45 [PATCH v2 for-4.10] ubsan: add clang 5.0 support Roger Pau Monne
2017-10-18 9:23 ` Jan Beulich
2017-10-18 9:42 ` Roger Pau Monné
2017-10-18 9:53 ` Jan Beulich
2017-10-18 9:58 ` Roger Pau Monné
2017-10-18 10:17 ` Wei Liu
2017-10-18 13:35 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.