I noticed that on EnterVT after a suspend valgrind would complain due to ->name being freed earlier, after X sometimes crashed on returning from suspend. Attached patch fixes this for me on x1.11 which seems to be identical to x-server.git in almost every way. It has been reapplied to X server
Alternatively this member could be made a static array instead of a pointer, but this would require a video abi bump. I'm still a bit unfamiliar to the core x server so any feedback is welcome. The specific valgrind error happens a lot later, long after the fact: ==30042== Invalid read of size 1 ==30042== at 0x4C2BFD4: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==30042== by 0x673E7F5: strdup (strdup.c:42) ==30042== by 0x29673D: XNFstrdup (utils.c:1119) ==30042== by 0x1CEC08: xf86DuplicateMode (xf86Modes.c:209) ==30042== by 0x1C865E: xf86CrtcSetModeTransform (xf86Crtc.c:276) ==30042== by 0x1C8F30: xf86SetDesiredModes (xf86Crtc.c:2674) ==30042== by 0x8C5710E: RADEONEnterVT (radeon_driver.c:6269) ==30042== by 0x1D115F: xf86RandR12EnterVT (xf86RandR12.c:1764) ==30042== by 0x193967: xf86Wakeup (xf86Events.c:527) ==30042== by 0x15A7FA: WakeupHandler (dixutils.c:428) ==30042== by 0x28DDA5: WaitForSomething (WaitFor.c:235) ==30042== by 0x156601: Dispatch (dispatch.c:366) ==30042== Address 0x98bebf1 is 1 bytes inside a block of size 9 free'd ==30042== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==30042== by 0x1AAA51: xf86DeleteMode (xf86Mode.c:1985) ==30042== by 0x1C7617: xf86ProbeOutputModes (xf86Crtc.c:1575) ==30042== by 0x1D06B3: xf86RandR12GetInfo12 (xf86RandR12.c:1538) ==30042== by 0x20285C: RRGetInfo (rrinfo.c:202) ==30042== by 0x20743D: rrGetScreenResources (rrscreen.c:337) ==30042== by 0x1568B0: Dispatch (dispatch.c:442) However, if you look you can find the problem happen a lot earlier, if you add free(mode->name[1]); to get where the allocation occured: ==30042== Invalid free() / delete / delete[] / realloc() ==30042== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==30042== by 0x1AAA48: xf86DeleteMode (xf86Mode.c:1984) ==30042== by 0x1C7617: xf86ProbeOutputModes (xf86Crtc.c:1575) ==30042== by 0x1D06B3: xf86RandR12GetInfo12 (xf86RandR12.c:1538) ==30042== by 0x20285C: RRGetInfo (rrinfo.c:202) ==30042== by 0x20743D: rrGetScreenResources (rrscreen.c:337) ==30042== by 0x1568B0: Dispatch (dispatch.c:442) ==30042== by 0x1456D9: main (main.c:288) ==30042== Address 0x98bebf1 is 1 bytes inside a block of size 9 alloc'd ==30042== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==30042== by 0x296638: XNFalloc (utils.c:1048) ==30042== by 0x8CBA432: RADEONFPNativeMode (radeon_modes.c:148) ==30042== by 0x8CBB333: RADEONProbeOutputModes (radeon_modes.c:528) ==30042== by 0x8CB6336: radeon_get_modes (radeon_output.c:1297) ==30042== by 0x1C7706: xf86ProbeOutputModes (xf86Crtc.c:1614) ==30042== by 0x1C9334: xf86InitialConfiguration (xf86Crtc.c:2399) ==30042== by 0x8C4DFE3: RADEONPreInit (radeon_driver.c:3203) ==30042== by 0x197137: InitOutput (xf86Init.c:599) ==30042== by 0x14550C: main (main.c:205) Signed-off-by: Maarten Lankhorst <[email protected]> --- diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c index 2c8878f..8531b7b 100644 --- a/hw/xfree86/modes/xf86Crtc.c +++ b/hw/xfree86/modes/xf86Crtc.c @@ -362,6 +362,8 @@ xf86CrtcSetModeTransform(xf86CrtcPtr crtc, DisplayModePtr mode, done: if (ret) { + crtc->mode.name = strdup(mode->name); + free(saved_mode.name); crtc->active = TRUE; if (scrn->pScreen) xf86CrtcSetScreenSubpixelOrder(scrn->pScreen); @@ -2417,6 +2419,7 @@ xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow) xf86CrtcPtr crtc = config->crtc[c]; crtc->enabled = FALSE; + free(&crtc->desiredMode.name); memset(&crtc->desiredMode, '\0', sizeof(crtc->desiredMode)); /* Set default gamma for all crtc's. */ /* This is done to avoid problems later on with cloned outputs. */ @@ -2437,6 +2440,7 @@ xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow) if (mode && crtc) { crtc->desiredMode = *mode; + crtc->desiredMode.name = strdup(mode->name); crtc->desiredRotation = output->initial_rotation; crtc->desiredX = output->initial_x; crtc->desiredY = output->initial_y; @@ -2613,6 +2617,7 @@ xf86SetDesiredModes(ScrnInfoPtr scrn) continue; /* Mark that we'll need to re-set the mode for sure */ + free(crtc->mode.name); memset(&crtc->mode, 0, sizeof(crtc->mode)); if (!crtc->desiredMode.CrtcHDisplay) { DisplayModePtr mode = @@ -2620,7 +2625,9 @@ xf86SetDesiredModes(ScrnInfoPtr scrn) if (!mode) return FALSE; + free(crtc->desiredMode.name); crtc->desiredMode = *mode; + crtc->desiredMode.name = strdup(mode->name); crtc->desiredRotation = RR_Rotate_0; crtc->desiredTransformPresent = FALSE; crtc->desiredX = 0; @@ -2759,7 +2766,9 @@ xf86SetSingleMode(ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation) if (!xf86CrtcSetModeTransform(crtc, crtc_mode, rotation, NULL, 0, 0)) ok = FALSE; else { + free(crtc->desiredMode.name); crtc->desiredMode = *crtc_mode; + crtc->desiredMode.name = strdup(crtc_mode->name); crtc->desiredRotation = rotation; crtc->desiredTransformPresent = FALSE; crtc->desiredX = 0; @@ -2854,6 +2863,7 @@ xf86DisableUnusedFunctions(ScrnInfoPtr pScrn) if (!crtc->enabled) { crtc->funcs->dpms(crtc, DPMSModeOff); + free(crtc->mode.name); memset(&crtc->mode, 0, sizeof(crtc->mode)); xf86RotateDestroy(crtc); crtc->active = FALSE; diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c index 59b6f82..48cef30 100644 --- a/hw/xfree86/modes/xf86RandR12.c +++ b/hw/xfree86/modes/xf86RandR12.c @@ -1216,7 +1216,9 @@ xf86RandR12CrtcSet(ScreenPtr pScreen, /* * Save the last successful setting for EnterVT */ + free(crtc->desiredMode.name); crtc->desiredMode = mode; + crtc->desiredMode.name = strdup(mode.name); crtc->desiredRotation = rotation; if (transform) { crtc->desiredTransform = *transform; _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
