Opened 5 years ago

Closed 5 years ago

#42949 closed defect (fixed)

elvis crash on Mavericks

Reported by: Crosmatron Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.2.1
Keywords: haspatch maintainer Cc: ryandesign (Ryan Schmidt)
Port: elvis

Description

When I execute elvis it returns:

Abort trap: 6

Compiling with -O2 seems to fix the problem.

Attachments (2)

Portfile-elvis.diff (330 bytes) - added by Crosmatron 5 years ago.
CFLAGS instead
elvis-ryandesign.diff (646 bytes) - added by ryandesign (Ryan Schmidt) 5 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 5 years ago by neverpanic (Clemens Lang)

Keywords: haspatch maintainer added

Ideally, elvis should use the CFLAGS from configure.cflags (since that already contains optimization flags). Since it seems the Makefile correctly uses the $(CFLAGS) variable build.env-append CFLAGS="${configure.cflags} [get_canonical_archflags cc]" should help (the archflag thing is so that elvis will be compiled for the architecture chosen by a MacPorts user in macports.conf).

See wiki:UsingTheRightCompiler#nonstandard-ports. Other Makefile variables should probably be patched appropriately aswell:

build.env-append CPPFLAGS="${configure.cppflags}" \
                 CFLAGS="${configure.cflags} [get_canonical_archflags cc]" \
                 LDFLAGS="${configure.ldflags} [get_canonical_archflags ld]" \
                 CXXFLAGS="${configure.cxxflags} [get_canonical_archflags cxx]" # this line only if the project contains C++ code

comment:2 Changed 5 years ago by Crosmatron

Thanks. That definitely pointed me in the right direction, but I went for setting CC instead of CFLAGS because that's where elvis normally puts its optimisation flag.

Changed 5 years ago by Crosmatron

Attachment: Portfile-elvis.diff added

CFLAGS instead

comment:3 Changed 5 years ago by Crosmatron

For reference, this is where the crash is happening for me. With the -O2 flag that elvis uses by default, this does not trigger.

3   libsystem_c.dylib             	0x00007fff838eec91 abort_report_np + 181
4   libsystem_c.dylib             	0x00007fff83912860 __chk_fail + 48
5   libsystem_c.dylib             	0x00007fff83912870 __chk_fail_overlap + 16
6   libsystem_c.dylib             	0x00007fff83912892 __chk_overlap + 34
7   libsystem_c.dylib             	0x00007fff83912a6c __strcpy_chk + 64
8   elvis                         	0x000000010fd825d9 apply + 4409 (calc.c:2096)

comment:4 Changed 5 years ago by ryandesign (Ryan Schmidt)

Cc: ryandesign@… added

I agree with the maintainer: since the project's non-autoconf Makefile already puts the optimization flag into CC, that's where we should put it. It also stuffs other CFLAGS and CPPFLAGS into CC. I'm not comfortable setting CFLAGS in the portfile to a value ("-I. -Iosunix -O2") that contains intimate knowledge of the build system and could change with future versions of the software, although there hasn't been a new version in over 10 years, so maybe that's a moot point.

The default value of CC for this build system is "cc -O". MacPorts base overrides this to be ${configure.cc}, in other words specifying no optimization flag, which apparently results in the crash. The suggestion is now to use "-O2" as the optimization flag. Is there a reason why we don't want to use the MacPorts default optimization flag value of "-Os"?

Since this is a runtime crash, not a build failure, the port's revision needs to be increased to rebuild the port.

The port is not using -arch flags nor does it have a universal variant. This would be an opportunity to fix that as well.

See my attached patch which works for me and addresses all of the above.

Changed 5 years ago by ryandesign (Ryan Schmidt)

Attachment: elvis-ryandesign.diff added

comment:5 Changed 5 years ago by Crosmatron

Thank ryan. -Os is good too, I just used -O2 because elvis does sans-macports. Your patch is much nicer than mine and works for me.

comment:6 in reply to:  5 Changed 5 years ago by ryandesign (Ryan Schmidt)

Resolution: fixed
Status: newclosed

Replying to crosma@…:

Thank ryan. -Os is good too, I just used -O2 because elvis does sans-macports. Your patch is much nicer than mine and works for me.

Ah right, I see now that -O2 is set by the configure script, overriding the -O in the Makefile.

I committed my patch in r118152.

Note: See TracTickets for help on using tickets.