Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is -m32 necessary? #57

Closed
czchen opened this issue Jun 10, 2015 · 17 comments
Closed

Is -m32 necessary? #57

czchen opened this issue Jun 10, 2015 · 17 comments
Assignees

Comments

@czchen
Copy link

czchen commented Jun 10, 2015

Hi,

I tried to build afdko in 64-bit Linux machine, but due to -m32 flag, the ELF binary is for 32-bits. I wonder, is this flag -m32 necessary? Can I build an afdko for 64-bits system only?

@miguelsousa
Copy link
Member

@czchen the 32-bit dependency was removed in the past (see #17, #21), but it caused problems (#48) so it got reverted (#50).

@czchen
Copy link
Author

czchen commented Jun 10, 2015

@miguelsousa For this issue #48, I think whether we use -m32 or not, the comparison itself shall be fixed. Let me prepare a PR for those.

@readroberts
Copy link
Contributor

The problem is that there is not just one comparison issue to fix. There is a lot of FDK code which assumes an 'int' is no more than 4 bytes - remember, most of dates from the 1990's. It will take a large effort to fix this. Maybe only one or two man-days to find as much as possible by code inspection, but I'd expect to need a man-week or two of testing and fixing. However, I agree we should leave this open an issue to fix. - just noting that it is not a trivial issue to fix.

@czchen
Copy link
Author

czchen commented Jun 10, 2015

I think Card8, Card16, Card32 shall use fixed size data type (ex: uint8_t, uint16_t, uint32_t) instead of non fixed size data type. Maybe I can send a PR for this first?

@anthrotype
Copy link
Member

There is a lot of FDK code which assumes an 'int' is no more than 4 bytes... It will take a large effort to fix this.

@readroberts any progress on this -m32 issue? Can we give a hand? I'd like to compile makeotf into a library and link that to a 64-bit Python extension module.

@anthrotype
Copy link
Member

Maybe you could create a new branch called "64bit" or something like that, in which to start carrying out the work and where others can contribute patches? that could eventually be merged back into master once it's done.

@czchen
Copy link
Author

czchen commented Nov 27, 2015

Is there any unit test for afdko? I think it shall have some test cases before doing the 64-bit migration.

anthrotype pushed a commit to anthrotype/afdko that referenced this issue Nov 27, 2015
…issues with 64-bit arch

When compiling on 64-bit without `-m32` flag, `sizeof(long)` can be 8 instead of 4.
The `long` type is used in several type definitions for the `Fixed` data type.
However, the latter is defined in the OpenType spec as a 32-bit number, hence of size 4.

Similarly, `unsigned long` is used here to define `LOffset`, but the OpenType spec defines
`ULONG` as a 32 bit unsigned number.

See adobe-type-tools#57.

```diff
+#include <stdint.h>

-typedef long Fixed;
+typedef int32_t Fixed;

-typedef long cffFixed;
+typedef int32_t cffFixed;

-typedef long hotFixed;
+typedef int32_t hotFixed;

-typedef unsigned long LOffset;
+typedef uint32_t LOffset;
```

Files changed:
makeotf_lib/api/cffread.h
makeotf_lib/api/hotconv.h
makeotf_lib/source/cffread/cffread.c
makeotf_lib/source/hotconv/common.h
@readroberts
Copy link
Contributor

I am ready to drop support for 32-bit systems, and build the AFDKO only for 64 bit systems. I just made a new branch '64bit', and would be very happy to have anyone contribute to this work.

The AFDKO does not have a test harness. That has been on my task list for some years. What I use is a set of Adobe fonts: I run a script that applies the AFDKO tools to the fonts, and compares the output with the last set. This marginally adequate solution has worked well enough that building a real test harness hasn't yet made it to the top of the task list. I hope to get to it next year.

@anthrotype
Copy link
Member

Thank you, Read! I'll send out my patches to the new 64bit branch once they are ready.

@HinTak
Copy link

HinTak commented Mar 29, 2016

+1 . I am hoping to scavenging some of the CFF type 2 cstr code into a shared library/ dylib / dll to add to improve CFF support in Font Validator ( HinTak/Font-Validator#13 )

@HinTak
Copy link

HinTak commented Aug 16, 2016

I do something like this:

find /usr/share/fonts  /usr/share/texlive/texmf-dist/fonts/ -type f -iname '*.[ot]t?' -exec mono bin/FontValidator.exe ... -file "{}" ... \;

to feed every *.otf and *.otc and *.ttf and *.ttc to FontVal for testing. That's about 3000 of those, on Fedora Linux - and depends on what those ... are, can take days or weeks to run.

Don't know how many of them are ot[cf]'s, if that's important at all. But you could perhaps do the same thing to look for bugs with -m32.

@readroberts
Copy link
Contributor

Thanks for the advice.

@miguelsousa
Copy link
Member

Fixed via #312 (and #271).

@derwind
Copy link

derwind commented Aug 8, 2018

Hi,

autohintexe seems to be still compiled as a ELF 32-bit LSB executable under 64-bit Ubuntu 18.04 LTS.
Indeed 'file `which autohintexe`' shows 'ELF 32-bit LSB executable' where the autohintexe is installed by 'pip install afdko' (2.7.2).

I checked the latest source codes and found that

./autohint/autohintlib/config/linux/gcc/gcc.mak:14:XFLAGS = -m32
./autohint/config/linux/gcc/gcc.mak:15:XFLAGS = -m32

by 'find . -name "*" | xargs grep -n m32 2> /dev/null' under afdko/c.

@miguelsousa
Copy link
Member

@derwind autohint was intentionally left out of the conversion to 64bit because it will be replaced by psautohint in the near future.

@cjchapman
Copy link
Contributor

@derwind Here's the repo for psautohint:
https://github.com/adobe-type-tools/psautohint

@derwind
Copy link

derwind commented Aug 8, 2018

@miguelsousa @cjchapman Thank you for your explanation! I understand the situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants