Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Fri, 2007-03-09 at 13:49 +0200, Panagiotis Papadakos wrote: > Yep, I think it is OK! Pushed, thanks. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
Yep, I think it is OK! On Friday 09 March 2007 10:46, Michel Dänzer wrote: > On Thu, 2007-03-08 at 19:07 +0200, Panagiotis Papadakos wrote: > > On Thursday 08 March 2007 17:05, Michel Dänzer wrote: > > > Any idea what's going on? The only situation where radeonSetCliprects > > > doesn't get called from radeonMakeCurrent is if neither the drawable > > > nor its stamp has changed... > > > > We have to call radeonSetCliprects(radeon), before > > r300UpdateViewportOffset(radeon->glCtx). > > Ah right, like this? -- Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thu, 2007-03-08 at 19:07 +0200, Panagiotis Papadakos wrote: > On Thursday 08 March 2007 17:05, Michel Dänzer wrote: > > > > Any idea what's going on? The only situation where radeonSetCliprects > > doesn't get called from radeonMakeCurrent is if neither the drawable nor > > its stamp has changed... > > We have to call radeonSetCliprects(radeon), before > r300UpdateViewportOffset(radeon->glCtx). Ah right, like this? -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer 0001-r300-Also-update-window-state-when-it-s-already-bou.patch Description: application/mbox - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV-- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thursday 08 March 2007 17:05, Michel Dänzer wrote: > > Any idea what's going on? The only situation where radeonSetCliprects > doesn't get called from radeonMakeCurrent is if neither the drawable nor > its stamp has changed... We have to call radeonSetCliprects(radeon), before r300UpdateViewportOffset(radeon->glCtx). -- Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thu, 2007-03-08 at 17:03 +0200, Panagiotis Papadakos wrote: > I still get invalid reads with your latest patch. Any idea what's going on? The only situation where radeonSetCliprects doesn't get called from radeonMakeCurrent is if neither the drawable nor its stamp has changed... -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
I still get invalid reads with your latest patch. On Thursday 08 March 2007 15:42, Michel Dänzer wrote: > On Thu, 2007-03-08 at 14:31 +0100, Michel Dänzer wrote: > > On Thu, 2007-03-08 at 14:25 +0200, Panagiotis Papadakos wrote: > > > > Calling radeonSetCliprects when stamp!=lastStamp also helps! > > > > > > Sorry meant radeon->lastStamp != driDrawPriv->lastStamp > > > > That'll usually be fine, but in theory the stamps could be identical > > when a different drawable was bound previously. > > > > How does this patch look? > > Or how about one that actually compiles and doesn't crash... -- Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thu, 2007-03-08 at 14:31 +0100, Michel Dänzer wrote:
> On Thu, 2007-03-08 at 14:25 +0200, Panagiotis Papadakos wrote:
> > > Calling radeonSetCliprects when stamp!=lastStamp also helps!
> > Sorry meant radeon->lastStamp != driDrawPriv->lastStamp
>
> That'll usually be fine, but in theory the stamps could be identical
> when a different drawable was bound previously.
>
> How does this patch look?
Or how about one that actually compiles and doesn't crash...
--
Earthling Michel Dänzer | http://tungstengraphics.com
Libre software enthusiast | Debian, X and DRI developer
diff --git a/src/mesa/drivers/dri/r300/radeon_context.c b/src/mesa/drivers/dri/r300/radeon_context.c
index d66f1dc..08eea00 100644
--- a/src/mesa/drivers/dri/r300/radeon_context.c
+++ b/src/mesa/drivers/dri/r300/radeon_context.c
@@ -273,10 +273,11 @@ GLboolean radeonMakeCurrent(__DRIcontextPrivate * driContextPriv,
&radeon->vbl_seq);
}
+ radeon->dri.readable = driReadPriv;
+
if (radeon->dri.drawable != driDrawPriv ||
- radeon->dri.readable != driReadPriv) {
+ radeon->lastStamp != driDrawPriv->lastStamp) {
radeon->dri.drawable = driDrawPriv;
- radeon->dri.readable = driReadPriv;
r300UpdateWindow(radeon->glCtx);
r300UpdateViewportOffset(radeon->glCtx);
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thu, 2007-03-08 at 14:25 +0200, Panagiotis Papadakos wrote:
> > Calling radeonSetCliprects when stamp!=lastStamp also helps!
> Sorry meant radeon->lastStamp != driDrawPriv->lastStamp
That'll usually be fine, but in theory the stamps could be identical
when a different drawable was bound previously.
How does this patch look?
> and radeon->lastStamp != driReadPriv->lastStamp.
That comparison makes no sense as the cliprects and stamps only apply to
the drawable bound for drawing.
--
Earthling Michel Dänzer | http://tungstengraphics.com
Libre software enthusiast | Debian, X and DRI developer
diff --git a/src/mesa/drivers/dri/r300/radeon_context.c b/src/mesa/drivers/dri/r300/radeon_context.c
index d66f1dc..75bd70f 100644
--- a/src/mesa/drivers/dri/r300/radeon_context.c
+++ b/src/mesa/drivers/dri/r300/radeon_context.c
@@ -274,9 +274,8 @@ GLboolean radeonMakeCurrent(__DRIcontextPrivate * driContextPriv,
}
if (radeon->dri.drawable != driDrawPriv ||
- radeon->dri.readable != driReadPriv) {
+ radeon->lastStamp != drawable->lastStamp) {
radeon->dri.drawable = driDrawPriv;
- radeon->dri.readable = driReadPriv;
r300UpdateWindow(radeon->glCtx);
r300UpdateViewportOffset(radeon->glCtx);
@@ -284,6 +283,8 @@ GLboolean radeonMakeCurrent(__DRIcontextPrivate * driContextPriv,
radeonSetCliprects(radeon);
}
+ radeon->dri.readable = driReadPriv;
+
_mesa_make_current(radeon->glCtx,
(GLframebuffer *) driDrawPriv->
driverPrivate,
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
> Calling radeonSetCliprects when stamp!=lastStamp also helps! Sorry meant radeon->lastStamp != driDrawPriv->lastStamp and radeon->lastStamp != driReadPriv->lastStamp. - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Thursday 08 March 2007 12:12, Michel Dänzer wrote: > Does calling radeonSetCliprects unconditionally before > _mesa_make_current help? I don't see a way for radeonMakeCurrent to be > sure DoBindContext didn't update the cliprects. Yep it helps. Calling radeonSetCliprects when stamp!=lastStamp also helps! Regards Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Wed, 2007-03-07 at 23:19 +0200, Panagiotis Papadakos wrote: > The problem is that I still get invalid reads when going from fullscreen mode > to window mode for the first time, with my OSG application. Hmm, then I guess it re-binds the context in the process of switching between fullscreen and windowed? Does calling radeonSetCliprects unconditionally before _mesa_make_current help? I don't see a way for radeonMakeCurrent to be sure DoBindContext didn't update the cliprects. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
The problem is that I still get invalid reads when going from fullscreen mode to window mode for the first time, with my OSG application. On Wednesday 07 March 2007 19:10, Michel Dänzer wrote: > On Wed, 2007-03-07 at 19:02 +0200, Panagiotis Papadakos wrote: > > On Friday 02 March 2007 16:41, Michel Dänzer wrote: > > > On Mon, 2007-02-26 at 07:09 +0200, Panagiotis Papadakos wrote: > > > > Well I think I have the correct patch for this. > > > > > > I'm afraid not; the test modification is incorrect. The test is an > > > optimization to avoid doing unnecessary work when the context is > > > already up to date wrt the drawables passed in and doesn't have > > > anything to do with the stamps. > > > > Well I think we should do this, because __driUtilUpdateDrawableInfo, > > is called when lastStamp != stamp, and although the drawables have not > > changed, we get new cliprects. So we have to call radeonSetCliprects, > > else we point to memory freed in DoBindContext. > > Right, just your test modification was wrong. This turned out to also > fix issues with apps that don't call glViewport by default, so I pushed > something based on it in the meantime, see > http://bugs.freedesktop.org/show_bug.cgi?id=9876 . Thanks. -- Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Wed, 2007-03-07 at 19:02 +0200, Panagiotis Papadakos wrote: > On Friday 02 March 2007 16:41, Michel Dänzer wrote: > > On Mon, 2007-02-26 at 07:09 +0200, Panagiotis Papadakos wrote: > > > Well I think I have the correct patch for this. > > > > I'm afraid not; the test modification is incorrect. The test is an > > optimization to avoid doing unnecessary work when the context is already > > up to date wrt the drawables passed in and doesn't have anything to do > > with the stamps. > > Well I think we should do this, because __driUtilUpdateDrawableInfo, > is called when lastStamp != stamp, and although the drawables have not > changed, we get new cliprects. So we have to call radeonSetCliprects, else > we point to memory freed in DoBindContext. Right, just your test modification was wrong. This turned out to also fix issues with apps that don't call glViewport by default, so I pushed something based on it in the meantime, see http://bugs.freedesktop.org/show_bug.cgi?id=9876 . Thanks. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Friday 02 March 2007 16:41, Michel Dänzer wrote: > On Mon, 2007-02-26 at 07:09 +0200, Panagiotis Papadakos wrote: > > Well I think I have the correct patch for this. > > I'm afraid not; the test modification is incorrect. The test is an > optimization to avoid doing unnecessary work when the context is already > up to date wrt the drawables passed in and doesn't have anything to do > with the stamps. > Well I think we should do this, because __driUtilUpdateDrawableInfo, is called when lastStamp != stamp, and although the drawables have not changed, we get new cliprects. So we have to call radeonSetCliprects, else we point to memory freed in DoBindContext. -- Papadakos Panagiotis - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
On Mon, 2007-02-26 at 07:09 +0200, Panagiotis Papadakos wrote: > Well I think I have the correct patch for this. I'm afraid not; the test modification is incorrect. The test is an optimization to avoid doing unnecessary work when the context is already up to date wrt the drawables passed in and doesn't have anything to do with the stamps. > We should call radeonSetCliprects inside radeonMakeCurrent to update to > the new pClipRects whenever lastStamps change. I think that makes sense though. Maybe the updates of radeon->lastStamp could also be moved into radeonSetCliprects and/or the common parts of radeonMakeCurrent and r300RegainedLock moved into a separate function called by both. BTW, the patch also has superfluous whitespace-only changes, which should be avoided. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
Well I think I have the correct patch for this. We should call
radeonSetCliprects inside radeonMakeCurrent to update to
the new pClipRects whenever lastStamps change.
Probably this should also be applied to r200. Haven't seen for other drivers.
Comments?
P.S.
Should I try to get an account?
On Friday 02 February 2007 19:36, Panagiotis Papadakos wrote:
> Well this patch is not correct since it leads to memory leaks, probably due
> to succeeding calls to dri_interface->getDrawableInfo. So I created the
> attached patch, but again valgrind warns for invalid reads.
>
> Anyone understands why?
>
> Thanks
>
> P.S. Sorry but I am new to the code!
>
> On Thursday 01 February 2007 19:19, Panagiotis Papadakos wrote:
> > Checking with valgrind I got some invalid reads. The message was like the
> > following:
> >
> > ==6988== Invalid read of size 4
> > ==6988==at 0x4B3C7FD: intersect_rect (radeon_state.c:61)
> > ==6988==by 0x4B3C9DA: radeonRecalcScissorRects (radeon_state.c:108)
> > ==6988==by 0x4B3CAEC: radeonUpdateScissor (radeon_state.c:131)
> > ==6988==by 0x4B3CD04: radeonEnable (radeon_state.c:205)
> > ==6988==by 0x4B4B1C1: r300Enable (r300_state.c:542)
> > ==6988==by 0x4D13827: _mesa_set_enable (enable.c:956)
> > ==6988==by 0x4D138A6: _mesa_Enable (enable.c:971)
> > ==6988==by 0x4769879: glEnable (glapitemp.h:1160)
> > ==6988==by 0x4613A5F:
> > osgUtil::RenderStage::drawImplementation(osg::RenderInfo&,
> > osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988==by
> > 0x4607658: osgUtil::RenderBin::draw(osg::RenderInfo&,
> > osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988==by
> > 0x46133BC: osgUtil::RenderStage::drawInner(osg::RenderInfo&,
> > osgUtil::RenderLeaf*&, bool&) (in /usr/lib/libosgUtil.so) ==6988==by
> > 0x4612E6C: osgUtil::RenderStage::draw(osg::RenderInfo&,
> > osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988== Address
> > 0x4AF585C is 4 bytes inside a block of size 8 free'd ==6988==at
> > 0x402303F: free (vg_replace_malloc.c:233)
> > ==6988==by 0x4BAF503: _mesa_free (imports.c:93)
> > ==6988==by 0x4B2FF84: __driUtilUpdateDrawableInfo (dri_util.c:430)
> > ==6988==by 0x4B2FD46: DoBindContext (dri_util.c:339)
> > ==6988==by 0x4B2FF00: driBindContext (dri_util.c:383)
> > ==6988==by 0x4735921: BindContextWrapper (glxext.c:1620)
> > ==6988==by 0x4735A53: MakeContextCurrent (glxext.c:1674)
> > ==6988==by 0x4735D7C: glXMakeCurrent (glxext.c:1796)
> > ==6988==by 0x47D8BB3: Producer::RenderSurface::makeCurrent(bool) (in
> > /usr/lib/libProducer.so) ==6988==by 0x47DEEC6:
> > Producer::Camera::_frame(bool) (in /usr/lib/libProducer.so) ==6988==
> > by 0x47DF75F: Producer::Camera::frame(bool) (in /usr/lib/libProducer.so)
> > ==6988==by 0x47E2589: Producer::CameraGroup::_singleThreadedFrame()
> > (in /usr/lib/libProducer.so)
> >
> > So I searched a bit, and I created the following patch, which touches
> > src/mesa/drivers/dri/common/dri_util.c, and more specifically
> > __driUtilUpdateDrawableInfo, calling _mesa_free for pdp->pClipRects and
> > pdp->pBackClipRects only inside if.
> >
> > Does this look sane?
> >
> > Valgrind does not warn anymore.
--
Papadakos Panagiotis
diff --git a/src/mesa/drivers/dri/r300/radeon_context.c b/src/mesa/drivers/dri/r300/radeon_context.c
index 3a6bde8..10a8d35 100644
--- a/src/mesa/drivers/dri/r300/radeon_context.c
+++ b/src/mesa/drivers/dri/r300/radeon_context.c
@@ -52,6 +52,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DE
#include "radeon_reg.h"
#include "r300_state.h"
+#include "radeon_state.h"
#include "utils.h"
#include "vblank.h"
@@ -272,13 +273,15 @@ GLboolean radeonMakeCurrent(__DRIcontext
&radeon->vbl_seq);
}
- if (radeon->dri.drawable != driDrawPriv ||
- radeon->dri.readable != driReadPriv) {
+ if(radeon->lastStamp != driDrawPriv->lastStamp ||
+ radeon->lastStamp != driReadPriv->lastStamp) {
radeon->dri.drawable = driDrawPriv;
radeon->dri.readable = driReadPriv;
- r300UpdateWindow(radeon->glCtx);
- r300UpdateViewportOffset(radeon->glCtx);
+ radeonSetCliprects(radeon);
+ radeon->lastStamp = driDrawPriv->lastStamp;
+ r300UpdateWindow( radeon->glCtx );
+ r300UpdateViewportOffset( radeon->glCtx );
}
_mesa_make_current(radeon->glCtx,
diff --git a/src/mesa/drivers/dri/r300/radeon_lock.c b/src/mesa/drivers/dri/r300/radeon_lock.c
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
Well this patch is not correct since it leads to memory leaks, probably due to
succeeding calls to dri_interface->getDrawableInfo. So I created the attached
patch, but again valgrind warns for invalid reads.
Anyone understands why?
Thanks
P.S. Sorry but I am new to the code!
On Thursday 01 February 2007 19:19, Panagiotis Papadakos wrote:
> Checking with valgrind I got some invalid reads. The message was like the
> following:
>
> ==6988== Invalid read of size 4
> ==6988==at 0x4B3C7FD: intersect_rect (radeon_state.c:61)
> ==6988==by 0x4B3C9DA: radeonRecalcScissorRects (radeon_state.c:108)
> ==6988==by 0x4B3CAEC: radeonUpdateScissor (radeon_state.c:131)
> ==6988==by 0x4B3CD04: radeonEnable (radeon_state.c:205)
> ==6988==by 0x4B4B1C1: r300Enable (r300_state.c:542)
> ==6988==by 0x4D13827: _mesa_set_enable (enable.c:956)
> ==6988==by 0x4D138A6: _mesa_Enable (enable.c:971)
> ==6988==by 0x4769879: glEnable (glapitemp.h:1160)
> ==6988==by 0x4613A5F:
> osgUtil::RenderStage::drawImplementation(osg::RenderInfo&,
> osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988==by
> 0x4607658: osgUtil::RenderBin::draw(osg::RenderInfo&,
> osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988==by
> 0x46133BC: osgUtil::RenderStage::drawInner(osg::RenderInfo&,
> osgUtil::RenderLeaf*&, bool&) (in /usr/lib/libosgUtil.so) ==6988==by
> 0x4612E6C: osgUtil::RenderStage::draw(osg::RenderInfo&,
> osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so) ==6988== Address
> 0x4AF585C is 4 bytes inside a block of size 8 free'd ==6988==at
> 0x402303F: free (vg_replace_malloc.c:233)
> ==6988==by 0x4BAF503: _mesa_free (imports.c:93)
> ==6988==by 0x4B2FF84: __driUtilUpdateDrawableInfo (dri_util.c:430)
> ==6988==by 0x4B2FD46: DoBindContext (dri_util.c:339)
> ==6988==by 0x4B2FF00: driBindContext (dri_util.c:383)
> ==6988==by 0x4735921: BindContextWrapper (glxext.c:1620)
> ==6988==by 0x4735A53: MakeContextCurrent (glxext.c:1674)
> ==6988==by 0x4735D7C: glXMakeCurrent (glxext.c:1796)
> ==6988==by 0x47D8BB3: Producer::RenderSurface::makeCurrent(bool) (in
> /usr/lib/libProducer.so) ==6988==by 0x47DEEC6:
> Producer::Camera::_frame(bool) (in /usr/lib/libProducer.so) ==6988==by
> 0x47DF75F: Producer::Camera::frame(bool) (in /usr/lib/libProducer.so)
> ==6988==by 0x47E2589: Producer::CameraGroup::_singleThreadedFrame() (in
> /usr/lib/libProducer.so)
>
> So I searched a bit, and I created the following patch, which touches
> src/mesa/drivers/dri/common/dri_util.c, and more specifically
> __driUtilUpdateDrawableInfo, calling _mesa_free for pdp->pClipRects and
> pdp->pBackClipRects only inside if.
>
> Does this look sane?
>
> Valgrind does not warn anymore.
--
Papadakos Panagiotis
--- src/mesa/drivers/dri/common/dri_util.c 2007-02-02 19:30:24.0 +0200
+++ src/mesa/drivers/dri/common/dri_util_new.c 2007-02-02 19:34:39.0 +0200
@@ -426,13 +426,11 @@
return;
}
-if (pdp->pClipRects) {
- _mesa_free(pdp->pClipRects);
-}
-
-if (pdp->pBackClipRects) {
- _mesa_free(pdp->pBackClipRects);
-}
+/* Temporary hold pointers, so that we don't leak
+ * when (*dri_interface->getDrawableInfo) returns True
+ */
+drm_clip_rect_t * tempPClipRects = pdp->pClipRects;
+drm_clip_rect_t * tempPBackClipRects = pdp->pBackClipRects;
DRM_SPINUNLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
@@ -448,14 +446,29 @@
/* Error -- eg the window may have been destroyed. Keep going
* with no cliprects.
*/
-pdp->pStamp = &pdp->lastStamp; /* prevent endless loop */
+
+ if (tempPClipRects) {
+ _mesa_free(tempPClipRects);
+ }
+
+ if (tempPBackClipRects) {
+ _mesa_free(tempPBackClipRects);
+ }
+
+ pdp->pStamp = &pdp->lastStamp; /* prevent endless loop */
pdp->numClipRects = 0;
pdp->pClipRects = NULL;
pdp->numBackClipRects = 0;
pdp->pBackClipRects = NULL;
}
else
- pdp->pStamp = &(psp->pSAREA->drawableTable[pdp->index].stamp);
+{
+ if(tempPClipRects)
+ _mesa_free(tempPClipRects);
+ if(tempPBackClipRects)
+ _mesa_free(tempPBackClipRects);
+pdp->pStamp = &(psp->pSAREA->drawableTable[pdp->index].stamp);
+}
DRM_SPINLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
-
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Invalid reads from intersect_rect in radeon_state.c of r300 driver. [PATCH included]
Checking with valgrind I got some invalid reads. The message was like the
following:
==6988== Invalid read of size 4
==6988==at 0x4B3C7FD: intersect_rect (radeon_state.c:61)
==6988==by 0x4B3C9DA: radeonRecalcScissorRects (radeon_state.c:108)
==6988==by 0x4B3CAEC: radeonUpdateScissor (radeon_state.c:131)
==6988==by 0x4B3CD04: radeonEnable (radeon_state.c:205)
==6988==by 0x4B4B1C1: r300Enable (r300_state.c:542)
==6988==by 0x4D13827: _mesa_set_enable (enable.c:956)
==6988==by 0x4D138A6: _mesa_Enable (enable.c:971)
==6988==by 0x4769879: glEnable (glapitemp.h:1160)
==6988==by 0x4613A5F:
osgUtil::RenderStage::drawImplementation(osg::RenderInfo&,
osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so)
==6988==by 0x4607658: osgUtil::RenderBin::draw(osg::RenderInfo&,
osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so)
==6988==by 0x46133BC: osgUtil::RenderStage::drawInner(osg::RenderInfo&,
osgUtil::RenderLeaf*&, bool&) (in /usr/lib/libosgUtil.so)
==6988==by 0x4612E6C: osgUtil::RenderStage::draw(osg::RenderInfo&,
osgUtil::RenderLeaf*&) (in /usr/lib/libosgUtil.so)
==6988== Address 0x4AF585C is 4 bytes inside a block of size 8 free'd
==6988==at 0x402303F: free (vg_replace_malloc.c:233)
==6988==by 0x4BAF503: _mesa_free (imports.c:93)
==6988==by 0x4B2FF84: __driUtilUpdateDrawableInfo (dri_util.c:430)
==6988==by 0x4B2FD46: DoBindContext (dri_util.c:339)
==6988==by 0x4B2FF00: driBindContext (dri_util.c:383)
==6988==by 0x4735921: BindContextWrapper (glxext.c:1620)
==6988==by 0x4735A53: MakeContextCurrent (glxext.c:1674)
==6988==by 0x4735D7C: glXMakeCurrent (glxext.c:1796)
==6988==by 0x47D8BB3: Producer::RenderSurface::makeCurrent(bool) (in
/usr/lib/libProducer.so)
==6988==by 0x47DEEC6: Producer::Camera::_frame(bool) (in
/usr/lib/libProducer.so)
==6988==by 0x47DF75F: Producer::Camera::frame(bool) (in
/usr/lib/libProducer.so)
==6988==by 0x47E2589: Producer::CameraGroup::_singleThreadedFrame() (in
/usr/lib/libProducer.so)
So I searched a bit, and I created the following patch, which touches
src/mesa/drivers/dri/common/dri_util.c,
and more specifically __driUtilUpdateDrawableInfo, calling _mesa_free for
pdp->pClipRects and pdp->pBackClipRects
only inside if.
Does this look sane?
Valgrind does not warn anymore.
--
Papadakos Panagiotis
--- src/mesa/drivers/dri/common/dri_util.c 2007-02-01 19:11:25.0 +0200
+++ src/mesa/drivers/dri/common/dri_util_new.c 2007-02-01 19:02:44.0 +0200
@@ -426,14 +426,6 @@
return;
}
-if (pdp->pClipRects) {
- _mesa_free(pdp->pClipRects);
-}
-
-if (pdp->pBackClipRects) {
- _mesa_free(pdp->pBackClipRects);
-}
-
DRM_SPINUNLOCK(&psp->pSAREA->drawable_lock, psp->drawLockID);
if (!__driFindDrawable(psp->drawHash, pdp->draw) ||
@@ -448,7 +440,16 @@
/* Error -- eg the window may have been destroyed. Keep going
* with no cliprects.
*/
-pdp->pStamp = &pdp->lastStamp; /* prevent endless loop */
+
+ if (pdp->pClipRects) {
+ _mesa_free(pdp->pClipRects);
+ }
+
+ if (pdp->pBackClipRects) {
+ _mesa_free(pdp->pBackClipRects);
+ }
+
+ pdp->pStamp = &pdp->lastStamp; /* prevent endless loop */
pdp->numClipRects = 0;
pdp->pClipRects = NULL;
pdp->numBackClipRects = 0;
-
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
