Menu
▾
▴
mesa3d-dev
|
From: Corbin S. <mos...@gm...> - 2010-03-10 14:48:03
|
I don't know the VBO code that well, so I'm asking you guys if this has ever come up before. http://bugs.winehq.org/show_bug.cgi?id=22000 Gallium and indexed rendering are not very happy with each other. I get some fairly solidly reliable segfaults with both a d3d9 DLL test (device.ok) and Civ4 (Steam version). Hardware is a Radeon R580 (X1900), driver is r300g from Mesa git. My current guess, based on the Mesa debug info I dumped, is that the indexed rendering code is slightly baked and maybe trusting the underlying GL driver a bit too much. get_arrays_bounds: Handling 2 attrs attr 0: stride 16 size 12 start (nil) end 0xfffffffc attr 1: stride 16 size 4 start 0xc end (nil) buffer range: (nil) 0xfffffffc range -4 max index 4294967295 So right here (from device.ok) we have interleaved userspace VBO, that is being prepped inside core Mesa. Two delightful things here; the first attr seems way off-base, it shouldn't dare be giving us bad pointers, and the second attr's pointers don't even line up! Compare to a sane program (Mesa's drawarrays): get_arrays_bounds: Handling 2 attrs attr 0: stride 16 size 12 start 0x8087020 end 0x808705c attr 1: stride 16 size 4 start 0x808702c end 0x8087060 buffer range: 0x8087020 0x8087060 range 64 max index 3 r300g doesn't really care. The kernel drops the rendering on the floor for a variety of reasons, not least being the ridiculous max_index. But then it segfaults, and I have zero idea why. I'd guess it's something to do with tossing around NULL pointers like candy, but I'm honestly not sure and I haven't really dug into the DLL code yet. -- Only fools are easily impressed by what is only barely beyond their reach. ~ Unknown Corbin Simpson <Mos...@gm...> |
|
From: Keith W. <ke...@vm...> - 2010-03-10 14:59:19
|
On Wed, 2010-03-10 at 06:47 -0800, Corbin Simpson wrote: > I don't know the VBO code that well, so I'm asking you guys if this > has ever come up before. > > http://bugs.winehq.org/show_bug.cgi?id=22000 > > Gallium and indexed rendering are not very happy with each other. I get some > fairly solidly reliable segfaults with both a d3d9 DLL test (device.ok) and > Civ4 (Steam version). Hardware is a Radeon R580 (X1900), driver is r300g from > Mesa git. > > My current guess, based on the Mesa debug info I dumped, is that the indexed > rendering code is slightly baked and maybe trusting the underlying GL driver a > bit too much. > > get_arrays_bounds: Handling 2 attrs > attr 0: stride 16 size 12 start (nil) end 0xfffffffc > attr 1: stride 16 size 4 start 0xc end (nil) > buffer range: (nil) 0xfffffffc range -4 max index 4294967295 > > So right here (from device.ok) we have interleaved userspace VBO, that is being > prepped inside core Mesa. Two delightful things here; the first attr seems way > off-base, it shouldn't dare be giving us bad pointers, and the second attr's > pointers don't even line up! In VBO rendering, the pointers are really just offsets into the VBO, so should be interpreted as numbers. The first attribute has what looks like a small negative number. I don't know if that's legal or not in GL - I suspect it is probably not legal and should be rejected in the mesa validation code. I suspect what is happening is wine is passing in some negative values and we aren't properly rejecting them in mesa. > Compare to a sane program (Mesa's drawarrays): > > get_arrays_bounds: Handling 2 attrs > attr 0: stride 16 size 12 start 0x8087020 end 0x808705c > attr 1: stride 16 size 4 start 0x808702c end 0x8087060 > buffer range: 0x8087020 0x8087060 range 64 max index 3 This doesn't look like VBO rendering though. These are regular vertex arrays, meaning these values are proper pointers. > r300g doesn't really care. It shouldn't have to - the expectation in gallium is that inputs have been validated. > The kernel drops the rendering on the floor for a > variety of reasons, not least being the ridiculous max_index. > > But then it segfaults, and I have zero idea why. I'd guess it's something to do > with tossing around NULL pointers like candy, but I'm honestly not sure and I > haven't really dug into the DLL code yet. It's great you found a bug, but why all the excitement? Just track it down and propose a fix. Mesa's pretty well debugged, but there are always going to be new issues. I'm not sure why it warrants this type of implicit criticism when you come across something new. Keith |
|
From: Brian P. <br...@vm...> - 2010-03-10 15:12:05
|
Keith Whitwell wrote: > On Wed, 2010-03-10 at 06:47 -0800, Corbin Simpson wrote: >> I don't know the VBO code that well, so I'm asking you guys if this >> has ever come up before. >> >> http://bugs.winehq.org/show_bug.cgi?id=22000 >> >> Gallium and indexed rendering are not very happy with each other. I get some >> fairly solidly reliable segfaults with both a d3d9 DLL test (device.ok) and >> Civ4 (Steam version). Hardware is a Radeon R580 (X1900), driver is r300g from >> Mesa git. >> >> My current guess, based on the Mesa debug info I dumped, is that the indexed >> rendering code is slightly baked and maybe trusting the underlying GL driver a >> bit too much. >> >> get_arrays_bounds: Handling 2 attrs >> attr 0: stride 16 size 12 start (nil) end 0xfffffffc >> attr 1: stride 16 size 4 start 0xc end (nil) >> buffer range: (nil) 0xfffffffc range -4 max index 4294967295 Can you post the patch you used to print this info, Corbin? >> So right here (from device.ok) we have interleaved userspace VBO, that is being >> prepped inside core Mesa. Two delightful things here; the first attr seems way >> off-base, it shouldn't dare be giving us bad pointers, and the second attr's >> pointers don't even line up! > > In VBO rendering, the pointers are really just offsets into the VBO, so > should be interpreted as numbers. > > The first attribute has what looks like a small negative number. I > don't know if that's legal or not in GL - I suspect it is probably not > legal and should be rejected in the mesa validation code. This would have come in via the 'ptr' argument to one of the glVertex/Color/etcPointer() functions. We never check for negative pointer values. > I suspect what is happening is wine is passing in some negative values > and we aren't properly rejecting them in mesa. > >> Compare to a sane program (Mesa's drawarrays): >> >> get_arrays_bounds: Handling 2 attrs >> attr 0: stride 16 size 12 start 0x8087020 end 0x808705c >> attr 1: stride 16 size 4 start 0x808702c end 0x8087060 >> buffer range: 0x8087020 0x8087060 range 64 max index 3 > > This doesn't look like VBO rendering though. These are regular vertex > arrays, meaning these values are proper pointers. > >> r300g doesn't really care. > > It shouldn't have to - the expectation in gallium is that inputs have > been validated. > > >> The kernel drops the rendering on the floor for a >> variety of reasons, not least being the ridiculous max_index. >> >> But then it segfaults, and I have zero idea why. I'd guess it's something to do >> with tossing around NULL pointers like candy, but I'm honestly not sure and I >> haven't really dug into the DLL code yet. > > It's great you found a bug, but why all the excitement? Just track it > down and propose a fix. Mesa's pretty well debugged, but there are > always going to be new issues. I'm not sure why it warrants this type > of implicit criticism when you come across something new. -Brian |
|
From: Corbin S. <mos...@gm...> - 2010-03-12 00:19:43
|
On Wed, Mar 10, 2010 at 7:11 AM, Brian Paul <br...@vm...> wrote:
>>> get_arrays_bounds: Handling 2 attrs
>>> attr 0: stride 16 size 12 start (nil) end 0xfffffffc
>>> attr 1: stride 16 size 4 start 0xc end (nil)
>>> buffer range: (nil) 0xfffffffc range -4 max index 4294967295
>
> Can you post the patch you used to print this info, Corbin?
Inlined:
diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
index 8a6e1ed..7ae152c 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -293,6 +293,8 @@ get_arrays_bounds(const struct st_vertex_program *vp,
const GLubyte *high_addr = NULL;
GLuint attr;
+ debug_printf("get_arrays_bounds: Handling %u attrs\n", vpv->num_inputs);
+
for (attr = 0; attr < vpv->num_inputs; attr++) {
const GLuint mesaAttr = vp->index_to_input[attr];
const GLint stride = arrays[mesaAttr]->StrideB;
@@ -301,6 +303,8 @@ get_arrays_bounds(const struct st_vertex_program *vp,
_mesa_sizeof_type(arrays[mesaAttr]->Type));
const GLubyte *end = start + (max_index * stride) + sz;
+ debug_printf("attr %u: stride %d size %u start %p end %p\n",
attr, stride, sz, start, end);
+
if (attr == 0) {
low_addr = start;
high_addr = end;
@@ -348,7 +352,7 @@ setup_interleaved_attribs(GLcontext *ctx,
const GLubyte *low, *high;
get_arrays_bounds(vp, vpv, arrays, max_index, &low, &high);
- /*printf("buffer range: %p %p %d\n", low, high, high-low);*/
+ printf("buffer range: %p %p range %d max index %u\n", low,
high, high-low, max_index);
offset0 = low;
if (userSpace) {
>>> So right here (from device.ok) we have interleaved userspace VBO, that is
>>> being
>>> prepped inside core Mesa. Two delightful things here; the first attr
>>> seems way
>>> off-base, it shouldn't dare be giving us bad pointers, and the second
>>> attr's
>>> pointers don't even line up!
>>
>> In VBO rendering, the pointers are really just offsets into the VBO, so
>> should be interpreted as numbers.
>>
>> The first attribute has what looks like a small negative number. I
>> don't know if that's legal or not in GL - I suspect it is probably not
>> legal and should be rejected in the mesa validation code.
>
> This would have come in via the 'ptr' argument to one of the
> glVertex/Color/etcPointer() functions. We never check for negative pointer
> values.
Just started digging into this again, finally have spare time (if only
for a few days.) I will hopefully have a followup soon.
~ C.
--
Only fools are easily impressed by what is only
barely beyond their reach. ~ Unknown
Corbin Simpson
<Mos...@gm...>
|
|
From: Corbin S. <mos...@gm...> - 2010-03-10 15:18:39
|
On Wed, Mar 10, 2010 at 6:59 AM, Keith Whitwell <ke...@vm...> wrote: > It's great you found a bug, but why all the excitement? Just track it > down and propose a fix. Mesa's pretty well debugged, but there are > always going to be new issues. I'm not sure why it warrants this type > of implicit criticism when you come across something new. I wasn't aware of any implicit criticism in my post. If I said something wrong, I'm sorry; I'm a little bit fried from studying for a final, and what started as a relaxing diversion from math review has kind of turned into a multi-package bug and increased my frustration more than just a bit. I'll check the specs later and see if there's validation code missing, and also see if Wine needs similar patches. Again, sorry and thanks. ~ C. -- Corbin Simpson <Mos...@gm...> |
|
From: Keith W. <ke...@vm...> - 2010-03-10 15:24:48
|
On Wed, 2010-03-10 at 07:18 -0800, Corbin Simpson wrote: > On Wed, Mar 10, 2010 at 6:59 AM, Keith Whitwell <ke...@vm...> wrote: > > It's great you found a bug, but why all the excitement? Just track it > > down and propose a fix. Mesa's pretty well debugged, but there are > > always going to be new issues. I'm not sure why it warrants this type > > of implicit criticism when you come across something new. > > I wasn't aware of any implicit criticism in my post. If I said > something wrong, I'm sorry; I'm a little bit fried from studying for a > final, and what started as a relaxing diversion from math review has > kind of turned into a multi-package bug and increased my frustration > more than just a bit. > > I'll check the specs later and see if there's validation code missing, > and also see if Wine needs similar patches. > > Again, sorry and thanks. No problem Corbin. I suspect this is because DX9 allows negative offset values in some surprising places. Basically I think you've hit two bugs - one wine is trying to send Mesa an illegal negative value instead of translating it away somehow, and secondly we are accepting it... Ketih |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X