Opened 18 years ago

Closed 15 years ago

Last modified 11 years ago

#9053 closed defect (fixed)

BUG gmp: crash in config.guess due to mfpvr instruction

Reported by: vincent-opdarw@… Owned by: gwright@…
Priority: Low Milestone:
Component: ports Version: 1.0
Keywords: Cc: vincent-opdarw@…, gwright@…, boeyms@…, ryandesign (Ryan Carsten Schmidt), MarcusCalhoun-Lopez (Marcus Calhoun-Lopez), cooljeanius (Eric Gallager)
Port: gmp

Description (last modified by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez))

On my Power Mac G5, a crash occurs in config.guess, due to a generated code that contains the mfpvr instruction. This crash can be noticed (and is then annoying) when CrashReporter's developer mode has been enabled.

I think this can be fixed by trapping the corresponding signal (SIGILL?), but I can't currently try because my Mac is under repair.

Attachments (2)

avoid-crash.diff (5.4 KB) - added by vinc17@… 16 years ago.
patch to avoid the crash in config.guess on PowerPC: keep the Mac OS X test only
patch-config.guess.diff (355 bytes) - added by vinc17@… 15 years ago.
minimal patch to avoid the crash when configuring gmp 4.3.1

Download all attachments as: .zip

Change History (36)

comment:1 Changed 18 years ago by vincent-opdarw@…

I forgot to say: the fix must be a DarwinPorts patch since upstream doesn't want this bug to be fixed (see gmp-bugs mailing-list, thread "crash in config.guess on Power Mac G5 under Mac OS X 10.4.6").

comment:2 Changed 18 years ago by gwright@…

Owner: changed from darwinports-bugs@… to gwright@…

I'll take a look.

-Greg

comment:3 Changed 18 years ago by gwright@…

Status: newassigned

comment:4 Changed 18 years ago by gwright@…

Vincent,

I have read the exchange in the gmp-bugs mailing list. Does this problem _prevent_ building a working gmp library if the CrashReporter's developer mode is disabled? Or is a proper error code returned by the shell and the configuration procedure continues?

Greg

comment:5 Changed 18 years ago by vincent-opdarw@…

(In reply to comment #3)

I have read the exchange in the gmp-bugs mailing list. Does this problem _prevent_ building a working gmp library if the CrashReporter's developer mode is disabled?

I think that the building goes on normally (ditto if CrashReporter's developer mode is enabled, except that one has this annoying window).

comment:6 Changed 18 years ago by pipping@…

Milestone: Port Bugs

comment:7 Changed 17 years ago by boeyms@…

Cc: vincent-opdarw@… gwright@… boeyms@… added

Adding reporter, assignee and myself to Cc: list. Can either of you confirm whether or not bug exists in the current version of the gmp port?

comment:8 Changed 17 years ago by vinc17@…

Yes, it still exists. BTW, several months ago, I tried to solve the problem by trapping the signal, but though the signal was trapped, I still got the CrashReporter window appearing.

comment:9 Changed 17 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added
Priority: ExpectedLow

Since the build does succeed, and the Crash Reporter window is merely an annoyance, fixing this is low-priority.

comment:10 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Description: modified (diff)

I recently (r45928) took over partial maintainer-ship of gmp.
As I understand the situation, this is an issue only when CrashReporter is enabled and the rare occasion when gmp is built.
Unless there are any objections, I will close this ticket as wontfix.

comment:11 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Cc: mcalhoun@… added

Cc Me!

comment:12 in reply to:  10 ; Changed 16 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to mcalhoun@…:

As I understand the situation, this is an issue only when CrashReporter is enabled and the rare occasion when gmp is built.
Unless there are any objections, I will close this ticket as wontfix.

Before you do that, could you report the problem to the developers of gmp, so they can decide whether they want to fix this?

comment:13 Changed 16 years ago by ryandesign (Ryan Carsten Schmidt)

Port: gmp added

comment:14 in reply to:  12 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Replying to ryandesign@…:

Before you do that, could you report the problem to the developers of gmp, so they can decide whether they want to fix this?

There is a discussion on the gmp mailing list on this subject
(http://gmplib.org/list-archives/gmp-bugs/2006-May/000424.html)

The developers seem very much against working on this issue.

comment:15 Changed 16 years ago by vinc17@…

FYI, I tried trapping SIGILL in the past, but this didn't fix the problem (I suppose that the kernel doesn't have a way to know that SIGILL is trapped...).

Now, I'm wondering... What MacPorts is supposed to do concerning binaries? Produce binaries that can only run on the local machine? But in such a case, this is a bit contrary to universal support. Or produce binaries that can run on any declared architecture? For instance, any PowerPC-based machine if PowerPC is declared. In the latter case, GMP's config.guess should be patched to avoid particular CPU detection (as a side effect, this would avoid the crash in config.guess). One could test to see whether binaries would be significantly slower.

comment:16 Changed 16 years ago by vinc17@…

Anyway even in the former case, the -mcpu flag should then be global to MacPorts. So, in any case, GMP's config.guess does something wrong (in addition to the crash) in MacPorts.

comment:17 in reply to:  15 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Replying to vinc17@…:

FYI, I tried trapping SIGILL in the past, but this didn't fix the problem (I suppose that the kernel doesn't have a way to know that SIGILL is trapped...).

Now, I'm wondering... What MacPorts is supposed to do concerning binaries? Produce binaries that can only run on the local machine? But in such a case, this is a bit contrary to universal support. Or produce binaries that can run on any declared architecture? For instance, any PowerPC-based machine if PowerPC is declared. In the latter case, GMP's config.guess should be patched to avoid particular CPU detection (as a side effect, this would avoid the crash in config.guess). One could test to see whether binaries would be significantly slower.

I think it is fair to say that the primary focus of MacPorts is the building of binaries to run on the machine doing the building.
Intel/PowerPC universal support depends almost entirely on the individual ports.
In the case of gmp, full universal support has proven to be difficult, which is why the muniversal PortGroup is now used.

configure scripts, as a general strategy, attempt to do things (compile code or run programs).
Often the scripts expect the attempts to fail and behave accordingly.
Correct me if I'm wrong, but the objection in this case is that the failure is registered in the OS as a crash instead of just a failure.
I can see how this is an annoyance, but fixing it seems like allot of effort for little reward.

I would still vote that this ticket be closed as "wontfix."

comment:18 in reply to:  16 ; Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Replying to vinc17@…:

GMP's config.guess does something wrong (in addition to the crash) in MacPorts.

Could you please clarify what else the config.guess doe wrong (other than the crash)?

comment:19 in reply to:  18 ; Changed 16 years ago by vinc17@…

Replying to mcalhoun@…:

Replying to vinc17@…:

GMP's config.guess does something wrong (in addition to the crash) in MacPorts.

Could you please clarify what else the config.guess doe wrong (other than the crash)?

Together with the configure.in, it provides a -mcpu option to gcc (though MacPorts seems to override this setting), going against the default target; MPN_PATH (see configure output) is also affected, and I don't know whether MacPorts does something about it. Such a configuration should be done at a higher level, e.g. at the system level (Apple has done some choice in the configuration of the gcc provided by Xcode) or, here, at the MacPorts level (there's no reason why only GMP would use a different target). Moreover the -force_cpusubtype_ALL option (that is used in the Portfile) is probably contradictory. Disabling GMP's CPU subtype detection is probably the best thing to do.

This is a bit like the ABI chosen by GMP, which can be different from the default ABI (chosen by Apple and MacPorts). Concerning this problem (much more critical), the Portfiles already fixes it:

if {![variant_isset universal]} {
    configure.env   ABI=32
}

comment:20 in reply to:  19 ; Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

I am sorry, but I am having difficulty putting my finger on the problem.
-force_cpusubtype_ALL is required on PPC architectures or else gmp won't work properly.
ABI=32 is required or else gmp will try to build a 64-bit library.

Other than that, the Portfile does not override any other settings and gmp builds as the developers intended (as far as I know).
I do not see why -mpcu would necessarily be a problem or why disabling the CPU subtype detection would be a good thing.

Just to help me understand, are we still talking about the crash in config.guess?

comment:21 in reply to:  20 ; Changed 16 years ago by vinc17@…

Replying to mcalhoun@…:

-force_cpusubtype_ALL is required on PPC architectures or else gmp won't work properly.

What is the cause? (I think it may be related to broken CPU subtype detection.)

ABI=32 is required or else gmp will try to build a 64-bit library.

Yes, this is what I'm saying: MacPorts fixes a bad choice from GMP. And I think it should do something similar concerning CPU subtype detection.

I do not see why -mpcu would necessarily be a problem or why disabling the CPU subtype detection would be a good thing.

Why? The gcc man page says:

       The subtype of the file created (like ppc7400 or ppc970 or i686) is
       determined by the flags that specify the ISA that GCC is targetting,
       like -mcpu or -march.  The -force_cpusubtype_ALL option can be used to
       override this.

So, avoiding CPU subtype detection and -mcpu in the first place would avoid the use of -force_cpusubtype_ALL (which may be buggy here since it does not control the correctness of assembly code, which GMP uses).

Just to help me understand, are we still talking about the crash in config.guess?

Not directly, but a fix to the above problem can avoid the crash as a side effect.

comment:22 in reply to:  21 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Replying to vinc17@…:

Replying to mcalhoun@…:

-force_cpusubtype_ALL is required on PPC architectures or else gmp won't work properly.

What is the cause? (I think it may be related to broken CPU subtype detection.)

In the configure script is the following:

          *-*-darwin*)
            # On Darwin we can use 64-bit instructions with a longlong limb,                                                                                                                              
            # but the chip still in 32-bit mode.                                                                                                                                                          
            # In theory this can be used on any OS which knows how to save                                                                                                                                
            # 64-bit registers in a context switch.                                                                                                                                                       
            #                                                                                                                                                                                             
            # Note that we must use -mpowerpc64 with gcc, since the                                                                                                                                       
            # longlong.h macros expect limb operands in a single 64-bit                                                                                                                                   
            # register, not two 32-bit registers as would be given for a                                                                                                                                  
            # long long without -mpowerpc64.  In theory we could detect and                                                                                                                               
            # accomodate both styles, but the proper 64-bit registers will                                                                                                                                
            # be fastest and are what we really want to use.                                                                                                                                              
            #                                                                                                                                                                                             
            # One would think -mpowerpc64 would set the assembler in the right                                                                                                                            
            # mode to handle 64-bit instructions.  But for that, also                                                                                                                                     
            # -force_cpusubtype_ALL is needed.  

So it is possible that -force_cpusubtype_ALL is required to get the PowePC to be in the correct state.
I admit I do not understand the intricacies of processor architectures.

I think the bottom line is that as far a I know, gmp builds and runs just as the developers intended.
If there is a particular patch, however, we can reopen the discussion.

comment:23 Changed 16 years ago by vinc17@…

However if GMP uses -mpowerpc64 with -force_cpusubtype_ALL, then it must not use -mcpu.

comment:24 Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Resolution: wontfix
Status: assignedclosed

If the CFLAGS and CXXFLAGS environmental variables are not set (as in r47780) the configure script chooses the fastest options it can find.
On a G5 machine -mpowerpc64, -force_cpusubtype_ALL, and -mcpu seem to coexist.
The choice is intention and does not seem to cause any problems.
I will therefore close this ticket.

If there is a speed issue or a specific error, we can revisit the issue.

comment:25 in reply to:  24 ; Changed 16 years ago by vinc17@…

Resolution: wontfix
Status: closedreopened

Replying to mcalhoun@…:

On a G5 machine -mpowerpc64, -force_cpusubtype_ALL, and -mcpu seem to coexist.

Yes, seem. But you haven't tried hard (such as testing the library, at least the 64-bit version, on a different PowerPC machine).

I will therefore close this ticket.

I'm reopening it since there's no reason not to fix the crash itself (which is annoying). I'm going to attach a patch.

Changed 16 years ago by vinc17@…

Attachment: avoid-crash.diff added

patch to avoid the crash in config.guess on PowerPC: keep the Mac OS X test only

comment:26 in reply to:  25 ; Changed 16 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Replying to vinc17@…:

Replying to mcalhoun@…:

On a G5 machine -mpowerpc64, -force_cpusubtype_ALL, and -mcpu seem to coexist.

Yes, seem. But you haven't tried hard (such as testing the library, at least the 64-bit version, on a different PowerPC machine).

I will clarify.
The test suite passed in 32 bit mode.
The test suite passed in 64 bit mode.
Since I only have access to the one G5 machine, such testing will have to suffice even if it does not qualify as having "tried hard."
Is there any reason to believe this is not correct?

I'm reopening it since there's no reason not to fix the crash itself (which is annoying). I'm going to attach a patch.

I would respectfully suggest that unless the patch is quite small, there is no reason for it.
Correct me if I am wrong, but the crash is part of a normal test process.
Code is run, it fails, and so the configure script knows not to run that code.
The only issue is the minor and infrequent annoyance of CrashReporter telling you about the failure.

comment:27 in reply to:  26 Changed 16 years ago by vinc17@…

Replying to mcalhoun@…:

The test suite passed in 32 bit mode.
The test suite passed in 64 bit mode.
Since I only have access to the one G5 machine, such testing will have to suffice even if it does not qualify as having "tried hard."
Is there any reason to believe this is not correct?

The correctness of arch-related options only matters if you try the binary on a different machine. So, if you test the library only on your machine, you're not doing any real test concerning these options.

I would respectfully suggest that unless the patch is quite small, there is no reason for it.

The patch is small (I could make it even smaller by changing only one line, e.g. with a reinplace in the Portfile, but I think that the way it is is cleaner and 5 KB is not too much).

Correct me if I am wrong, but the crash is part of a normal test process.

No, this is not normal. The test in question (where the crash occurs) is designed for Linux. I wonder why it isn't disabled on other systems (at least on those where this particular test is known to fail, as documented).

Code is run, it fails, and so the configure script knows not to run that code.

No. The configure script runs config.guess, but only uses its output. But removing the mfpvr test doesn't change the config.guess output since this test fails to give any information on the processor. Under Mac OS X, the information on the processor is given by the test designed for Mac OS X (or more generally Mach-O).

Note: it is theoretically possible to make the AIX test "work" and return an incorrect result (thus making the GMP build fail) by adding a sys/systemcfg.h header to the system (I don't think there's anything wrong with that, because sys/systemcfg.h isn't standard under Mac OS X thus well-written software shouldn't use it), even though this is quite unlikely in practice (but remember that there was a real problem with malloc.h, and this was quite similar). However since there's a patch, it was the occasion to remove the second buggy test.

comment:28 Changed 16 years ago by (none)

Milestone: Port Bugs

Milestone Port Bugs deleted

Changed 15 years ago by vinc17@…

Attachment: patch-config.guess.diff added

minimal patch to avoid the crash when configuring gmp 4.3.1

comment:29 Changed 15 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Please forgive me since I know you have tried to explain this before, but I still do not see why a patch is needed at all (even if it is small).
I have tested gmp on a G5, and config.guess gives the correct result.

config.guess first tries a Linux only test (which fails), then it tries an AIX only test (which fails), and finally tries a Mac OS X test (which works).
Granted, the Linux test fails less gracefully because it uses the mfpvr instruction, but why is that a problem?

comment:30 in reply to:  29 Changed 15 years ago by vinc17@…

Replying to mcalhoun@…:

I have tested gmp on a G5, and config.guess gives the correct result.

It gives the correct result, but makes a dialog box appear (because of the crash), which is really annoying, in particular when one does something else while upgrading.

comment:31 Changed 15 years ago by ryandesign (Ryan Carsten Schmidt)

Remember, the dialog box only appears on screen if you have opened /Developer/Applications/Utilities/CrashReporterPrefs.app and changed the mode from Basic to Developer. (I have done this, and recommend other MacPorts developers do too.)

comment:32 Changed 15 years ago by vinc17@…

Yes, this is what I've said in my bug report (see the end of the first paragraph).

comment:33 Changed 15 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Resolution: fixed
Status: reopenedclosed

Fixed in r51706.

comment:34 Changed 11 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

Note: See TracTickets for help on using tickets.