Commit 99e22b86c5f1a3653f3caaf01368a777d2b208d0 from Adam Jackson introduced a build failure for me. Gcc (4.5.3) complains that the access to EstIIIModes from DDCModesFromEstIII() goes beyound the end of that array. And I think gcc is right. DDCModesFromEstIII() loops i until 5 and j until 1, so the maximum index will be (5*8)+(7-1) == 46, but EstIIIModes[] only contains 44 elements. So if the last bits of est are set, then DDCModesFromEstIII() will read garbage from beyond the end of the array. I suspect these bits are reserved in the standard, because there are no modes defined for them, but that does not prevent a monitor vendor from shipping a broken EDID. So I would suggest filling the mode array with 4 additional dummy modes. Hmmh: for (j = 7; j > 0; j--) Shouldn't this be j >= 0? Otherwise the lowest bit will never be checked. (Modes "1152, 864, 75", "1280, 1024, 85", "1400, 1050, 75", ...)
A patch was posted here http://lists.x.org/archives/xorg-devel/2011-November/027176.html but it seems that nobody replied.
Created attachment 56770 [details] [review] Fix access to EstIIIModes[] I would prefer this patch. It solves my compile error and the resulting X server works for me. (I'm using it to post this comment...)
(In reply to comment #2) > Created attachment 56770 [details] [review] [review] > Fix access to EstIIIModes[] > > I would prefer this patch. > It solves my compile error and the resulting X server works for me. (I'm using it to post this comment...) This patch is cleaner and corrects the off-by-one error, whereas the patch given in comment #1 does not.
Is this still an issue for you? If so, please send your patch to xorg-devel for review.
Yes, the issue still exists and it is for example popping up in Gentoo: https://bugs.gentoo.org/show_bug.cgi?id=405139 https://bugs.gentoo.org/show_bug.cgi?id=420127 The Gentoo-Fix for 1.11 is currently to automatically downgrade from -O3 to -O2, for 1.12 they added my patch. I will post it to the mailing list.
Patch send to mailing list 14 days ago: http://lists.freedesktop.org/archives/xorg-devel/2012-June/031660.html No reaction, but I hope that means nobody has (serious) problems with it.
Please followup on the list. I've been on honeymoon for the past 2 weeks and have quite a backlog to churn through. Hopefully someone can get to it before I do.
Created attachment 63875 [details] [review] Fix above array bounds error Split the two issues into separate patches, suggesting from Peter Hutterer on the xorg-devel ml.
Created attachment 63876 [details] [review] Patch for the second issue: the skipped modes
Comment on attachment 63875 [details] [review] Fix above array bounds error Review of attachment 63875 [details] [review]: ----------------------------------------------------------------- I'd slightly prefer to do this by bounds-checking m itself, but this is fine too. Reviewed-by: Adam Jackson <ajax@redhat.com>
Comment on attachment 63876 [details] [review] Patch for the second issue: the skipped modes Review of attachment 63876 [details] [review]: ----------------------------------------------------------------- Reviewed-by: Adam Jackson <ajax@redhat.com>
Patches have been committed to the xserver git tree, closing as RESOLVED/FIXED Thanks everyone!
commit f27fcb81c4a30cec899628e4bb3e300edbcebe4b and commit 738e55ebbdf516a45b95761b815bed4e697dc726
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.