bugThe FreeType Project - Bugs: bug #23556, FT_MulFix clobbers EDX register...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #23556: FT_MulFix clobbers EDX register without telling gcc

Submitter:  None
Submitted:  Thu 12 Jun 2008 07:31:06 PM UTC
   
 
Severity:  3 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  wl Open/Closed:  Closed
Planned Release:  2.3.7

Sun 15 Jun 2008 11:09:26 AM UTC, comment #3: 

Thanks a lot for the analysis and the patch.  I've applied it to the CVS.

Interestingly, with gcc 4.1.2 (and -O3), I don't see problems with the EAX register (see attached files); the attached program gives

bad: 31 0
bad: 31 46
good: 31 46
good: 31 46


(file #15846, file #15847)

Werner LEMBERG <wl>
Group administrator
Sun 15 Jun 2008 01:16:21 AM UTC, comment #2: 

The patch gave me the compiler error "error: can't find a register in class `DREG' while reloading `asm'". gcc does not seem to allow putting the same register as both an input and a clobber. I think it needs to be an output operand.

Actually now that I have looked at the code again, I see that the same issue exists for EAX. This does not affect ft_recompute_scaled_metrics but it maybe could cause problems elsewhere.

Here is some test code to demonstrate. (Put the FT_MulFix code in the same file, so that gcc can inline it)

FT_Long x = FT_MulFix(2000, 1000);
FT_Long y = FT_MulFix(3000, 1000); // gcc thinks EDX is still 1000
printf("%d %d ", x, y);

x = FT_MulFix(1000, 2000);
y = FT_MulFix(1000, 3000); // gcc thinks EAX is still 1000
printf("%d %d ", x, y);

Compile with -O3, to enable automatic inlining, and it prints out 31 0 31 1, rather than the expected 31 46 31 46.

I changed the constraints to:

      : "=a"(result), "+d"(b)
      : "a"(a)
      : "%ecx"

and that fixes it.

Anonymous
Sat 14 Jun 2008 06:37:28 AM UTC, comment #1: 

Does the patch below fix the issue?

Can you provide a test case/font/etc., where you have observed this erroneous behaviour?


--- ftcalc.c    2008-05-08 22:44:41.000000000 +0200
+++ ftcalc.c.new        2008-06-14 08:35:20.000000000 +0200
@@ -432,7 +432,7 @@
       "mov   %%eax, %0\n"
       : "=r"(result)
       : "a"(a), "d"(b)
-      : "%ecx"
+      : "%ecx", "%edx"
     );
     return result;

Werner LEMBERG <wl>
Group administrator
Thu 12 Jun 2008 07:31:06 PM UTC, original submission:  

The assembly code for FT_MulFix does not tell gcc that it changes the EDX register. If function inlining is enabled, this causes a problem: when FT_MulFix is called inline from ft_recompute_scaled_metrics, gcc incorrectly thinks that EDX still holds metrics->y_scale, and the descender size is calculated wrong (usually, as 0).

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #15846:  mulfix.c added by wl (1KiB - text/x-csrc)
file #15847:  mulfix.s added by wl (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by wl (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-06-15 wl StatusNeed Info Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 2.3.7
    2008-06-15 wl Attached File- Added mulfix.c, #15846
        Attached File- Added mulfix.s, #15847
    2008-06-14 wl StatusNone Need Info
        Assigned toNone wl

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code