| Message ID | 1463120076-9253-1-git-send-email-nhaehnle@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Nicolai, On 13 May 2016 at 07:14, Nicolai Hähnle <nhaehnle@gmail.com> wrote: > From: Nicolai Hähnle <nicolai.haehnle@amd.com> > > Previously, (*device)->businfo.pci would end up misaligned, which results > in undefined behavior. > Can you point me to a source where I can read more on the topic ? I'm pretty sure I ran this through valgrind and it gave a clear bill of health. Thanks Emil P.S. Please run the following in your repo $git config --local format.subjectPrefix "PATCH libdrm"
Hi Emil, On 14.05.2016 18:03, Emil Velikov wrote: > Hi Nicolai, > > On 13 May 2016 at 07:14, Nicolai Hähnle <nhaehnle@gmail.com> wrote: >> From: Nicolai Hähnle <nicolai.haehnle@amd.com> >> >> Previously, (*device)->businfo.pci would end up misaligned, which results >> in undefined behavior. >> > Can you point me to a source where I can read more on the topic ? > I'm pretty sure I ran this through valgrind and it gave a clear bill of health. Valgrind doesn't complain here either, I noticed it with -fsanitize=undefined (ubsan) applied to Mesa. That makes sense, since unaligned loads/stores aren't really a bug as far as the x86 ISA is concerned, which is the level at which valgrind looks at the code. On the other hand, apparently already an unaligned cast (not just the dereference!) is undefined behavior as far as C is concerned. I'm not a C language lawyer, but that's what people are saying on the interwebs, so it must be true :) > P.S. Please run the following in your repo $git config --local > format.subjectPrefix "PATCH libdrm" Done. Cheers, Nicolai
Ping. On 13.05.2016 08:14, Nicolai Hähnle wrote: > From: Nicolai Hähnle <nicolai.haehnle@amd.com> > > Previously, (*device)->businfo.pci would end up misaligned, which results > in undefined behavior. > > Signed-off-by: Nicolai Hähnle <nicolai.haehnle@amd.com> > --- > xf86drm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xf86drm.c b/xf86drm.c > index 5f587d9..94efc08 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -3000,7 +3000,7 @@ static int drmProcessPciDevice(drmDevicePtr *device, const char *d_name, > const char *node, int node_type, > int maj, int min, bool fetch_deviceinfo) > { > - const int max_node_str = drmGetMaxNodeName(); > + const int max_node_str = ALIGN(drmGetMaxNodeName(), sizeof(void *)); > int ret, i; > char *addr; > >
On 10 June 2016 at 15:07, Nicolai Hähnle <nhaehnle@gmail.com> wrote: > Ping. > Sorry about that, I thought I've already mentioned it: Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> I take it you have commit access ? If not, let me know and I'll push if for you. Emil
On 10.06.2016 18:55, Emil Velikov wrote: > On 10 June 2016 at 15:07, Nicolai Hähnle <nhaehnle@gmail.com> wrote: >> Ping. >> > Sorry about that, I thought I've already mentioned it: > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> > > I take it you have commit access ? If not, let me know and I'll push if for you. Thanks, I just pushed it. Cheers, Nicolai > Emil >
diff --git a/xf86drm.c b/xf86drm.c index 5f587d9..94efc08 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -3000,7 +3000,7 @@ static int drmProcessPciDevice(drmDevicePtr *device, const char *d_name, const char *node, int node_type, int maj, int min, bool fetch_deviceinfo) { - const int max_node_str = drmGetMaxNodeName(); + const int max_node_str = ALIGN(drmGetMaxNodeName(), sizeof(void *)); int ret, i; char *addr;