Ticket #758 (closed bug: fixed)

Opened 6 years ago

Last modified 5 years ago

Fix problems on openbsd/hppa

Reported by: reezer Owned by: bacek
Priority: normal Milestone:
Component: none Version: 1.2.0
Severity: medium Keywords: hppa numification
Cc: Language:
Patch status: Platform: openbsd

Description

Hi,

I have been running smoke tests for openbsd/hppa for a while, but I'm not sure if someone is even interested in them or if I should change some configure arguments or something else.

You can find the smoke reports on:  http://smolder.plusthree.com/app/public_projects/smoke_reports/8?tag=Perl%205.10.0%20hppa-openbsd

A new run is done every half hour, if there are changes.

Attachments

str_to_num.patch Download (0.8 KB) - added by bacek 6 years ago.
Possible workaround for str_to_num
tryfloatval_mod.c Download (463 bytes) - added by doughera 6 years ago.

Change History

  Changed 6 years ago by Infinoid

  • keywords hppa numification added
  • owner set to bacek

Thanks for the report! It looks like r39529 (bacek's tt24_unicode_numifications merge) was the culprit. Assigning ticket...

  Changed 6 years ago by bacek

Hello.

Can you try build parrot with this patch:

--- a/src/string/api.c
+++ b/src/string/api.c
@@ -2316,16 +2316,16 @@ Parrot_str_to_num(PARROT_INTERP, ARGIN(const STRING *s))
     }
 
     if (d && d_is_safe) {
-        f = mantissa + (1.0 * d / powl(10, d_length));
+        f = mantissa + (1.0 * d / pow(10, d_length));
     }
 
     f = f * sign;
 
     if (e) {
         if (e_sign == 1)
-            f *= powl(10, e);
+            f *= pow(10, e);
         else
-            f /= powl(10, e);
+            f /= pow(10, e);
     }
 
     return f;

I suspect "powl" doing something different on hppa.

-- Bacek

Changed 6 years ago by bacek

Possible workaround for str_to_num

  Changed 6 years ago by reezer

follow-ups: ↓ 5 ↓ 6   Changed 6 years ago by bacek

It is with "str_to_num.patch" applied?

in reply to: ↑ 4   Changed 6 years ago by Infinoid

Replying to bacek:

It is with "str_to_num.patch" applied?

I think it must be. I think  http://smolder.plusthree.com/app/public_projects/report_details/23680 is r39537 without the patch,  http://smolder.plusthree.com/app/public_projects/report_details/23684 is r39537 with the patch.

So after the patch, there are two additional regressions, but all the other failures are fixed.

 http://smolder.plusthree.com/app/public_projects/tap_stream/23684/125 looks like something else (a different configuration option?).  http://smolder.plusthree.com/app/public_projects/tap_stream/23684/197 looks related to some of today's other changes.

Mark

in reply to: ↑ 4 ; follow-up: ↓ 8   Changed 6 years ago by reezer

Replying to bacek:

It is with "str_to_num.patch" applied?

yes

Anything else I should test?

  Changed 6 years ago by jkeenan

  • summary changed from Fix problems o openbsd/hppa to Fix problems on openbsd/hppa

in reply to: ↑ 6   Changed 6 years ago by bacek

Replying to reezer:

yes

Thanks a lot! Patch applied at r39565.

Anything else I should test?

Can you test current trunk with this patch? AFAIU it should resolve test failures in t/op/arithmetics.t

--- a/src/ops/math.ops
+++ b/src/ops/math.ops
@@ -488,7 +488,7 @@ inline op fdiv(out INT, in INT, in INT) :base_core {
     goto ADDRESS(handler);
   }
 
-  f  = floor($2 / den);
+  f  = floor((FLOATVAL)$2 / (FLOATVAL)den);
   $1 = (INTVAL)f;
 }
 

-- Bacek

follow-up: ↓ 10   Changed 6 years ago by reezer

in reply to: ↑ 9   Changed 6 years ago by bacek

Replying to reezer:

Sorry for the delay. Here the result:  http://smolder.plusthree.com/app/public_projects/report_details/23871

Ok... I'm out of ideas why those tests failing.

-- Bacek

follow-up: ↓ 12   Changed 6 years ago by doughera

I'm having trouble following this report, since the various links take me to pages with many smolder reports, and I have to click through each one to try to guess what specific issue is being discussed. (It doesn't help that even when I can get through to Smolder, some of the links on Smolder come back

Error
You shouldn't be here. Consider yourself warned.

Anyway, the patch pasted inline above is to fdiv(), while the most recent failures in t/op/arithmetics.t are in test 11, which uses floor(), not fdiv(), so I'm not sure what I'm missing.

Before guessing further, however, two things would be *very* useful:

1. A posting *here* (either attached or inline if it's brief) of specific output of specific tests that fail (with all details about specific parrot version and any local patches clearly specified) and

2. A posting *here* (either attached or inline) of the ./myconfig file generated by parrot's Configure.pl.

Thank you.

in reply to: ↑ 11 ; follow-up: ↓ 13   Changed 6 years ago by reezer

Replying to doughera: (It doesn't help that even when I can get through to Smolder, some of the links on Smolder come back

{{{ Error You shouldn't be here. Consider yourself warned. }}}

This is offt-topic, but I guess that this one has something to do with diabled javascipt or javascript, which isn't completely loaded or interpreted correctly.

1. A posting *here* (either attached or inline if it's brief) of specific output of specific tests that fail (with all details about specific parrot version and any local patches clearly specified) and

The current problem is with a trunk version (r39630) and baceks math.ops patch. I reply to the posts with patches to make clear which of them have been used (and bacek replied to the specific test results). But you're right it's easier with more precise information.

not ok 10 - mod_n
#   Failed test 'mod_n'
#   at t/op/number.t line 299.
#                   '5
# 0
# 2
# 2
# -2
# -2
# '
#     doesn't match '/5
# -?0
# 2
# -1
# 1
# -2
# /
# '

2. A posting *here* (either attached or inline) of the ./myconfig file generated by parrot's Configure.pl.

The used configs (at least the ones given on the command line) are part of the reports, but here they are:

Summary of my parrot 1.3.0 (r39630) configuration:
  configdate='Tue Jun 16 09:40:50 2009 GMT'
  Platform:
    osname=openbsd, archname=hppa-openbsd
    jitcapable=0, jitarchname=nojit,
    jitosname=openbsd, jitcpuarch=hppa
    execcapable=0
    perl=perl
  Compiler:
    cc='cc', ccflags=' -fno-delete-null-pointer-checks -pipe -I/usr/local/include -pthread -DHASATTRIBUTE_CONST  -DHASATTRIBUTE_DEPRECATED  -DHASATTRIBUTE_MALLOC  -DHASATTRIBUTE_NONNULL  -DHASATTRIBUTE_NORETURN  -DHASATTRIBUTE_PURE  -DHASATTRIBUTE_UNUSED  -falign-functions=16 -W -Wall -Waggregate-return -Wcast-align -Wcast-qual -Wchar-subscripts -Wcomment -Wdisabled-optimization -Wendif-labels -Wformat -Wformat-extra-args -Wformat-nonliteral -Wformat-security -Wformat-y2k -Wimplicit -Wimport -Winline -Wmissing-braces -Wno-missing-format-attribute -Wpacked -Wparentheses -Wpointer-arith -Wreturn-type -Wsequence-point -Wno-shadow -Wsign-compare -Wstrict-aliasing -Wswitch -Wswitch-default -Wtrigraphs -Wundef -Wunknown-pragmas -Wno-unused -Wwrite-strings -Wbad-function-cast -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wnonnull',
  Linker and Libraries:
    ld='cc', ldflags='-Wl,-E ',
    cc_ldflags='',
    libs='-lm -lutil -lpthread -lreadline -lncurses'
  Dynamic Linking:
    share_ext='.so', ld_share_flags='-shared -fPIC ',
    load_ext='.so', ld_load_flags='-shared -fPIC '
  Types:
    iv=long, intvalsize=4, intsize=4, opcode_t=long, opcode_t_size=4,
    ptrsize=4, ptr_alignment=4 byteorder=4321,
    nv=double, numvalsize=8, doublesize=8, longdoublesize=8

Oh and I'm sorry, if I miss everything. I'm only a hobby perl scripter and liked the ideas (the ones I understood) and thought I could help with testing perl on a not so usual hardware/os combination.

in reply to: ↑ 12   Changed 6 years ago by doughera

Replying to reezer:

Replying to doughera:

1. A posting *here* (either attached or inline if it's brief) of specific output of specific tests that fail (with all details about specific parrot version and any local patches clearly specified) and

The current problem is with a trunk version (r39630) and baceks math.ops patch. I reply to the posts with patches to make clear which of them have been used (and bacek replied to the specific test results). But you're right it's easier with more precise information.

not ok 10 - mod_n
#   Failed test 'mod_n'
#   at t/op/number.t line 299.

Thank you. The smolder posting was showing a failure in t/op/arithmetics.t test 11, which is different from this test. (Of course I can't go back and check it because I get 'Internal Server Error' from smolder for that report. I can read other, more recent, reports just fine (modulo the "you have been warned" errors) but I can't fetch this one.) Anyway, thank you. I now know which test is failing.

2. A posting *here* (either attached or inline) of the ./myconfig file generated by parrot's Configure.pl.

The used configs (at least the ones given on the command line) are part of the reports, but here they are:

> Summary of my parrot 1.3.0 (r39630) configuration:
>   configdate='Tue Jun 16 09:40:50 2009 GMT'
>   Platform:
>     osname=openbsd, archname=hppa-openbsd

>   Types:
>     iv=long, intvalsize=4, intsize=4, opcode_t=long, opcode_t_size=4,
>     ptrsize=4, ptr_alignment=4 byteorder=4321,
>     nv=double, numvalsize=8, doublesize=8, longdoublesize=8
> 

Again, thank you. I was particularly interested in the last line there, which shows the type of 'FLOATVAL' (called 'nv' in the output) you are using. You are using the normal 'double', which matches the expected arguments to floor(). (It is also possible for FLOATVAL to be 'long double' ,in which case you have to be particularly careful. That's not the issue here.)

It's hard to see what's gone wrong. Before looking for obscure bugs in parrot, it's perhaps worthwhile to verify that the relevant functions are working correctly on openbsd-hppa. Could you please try compiling the attached program tryfloatval_mod.c with the same compiler flags you used for parrot and see if it works? The output should be

floor( 5.0/-3.0) = -2.0, floatval_mod( 5.0, -3.0) = -1.0,  expect -1.0
floor(-5.0/ 3.0) = -2.0, floatval_mod(-5.0,  3.0) =  1.0,  expect  1.0
floor(-5.0/-3.0) =  1.0, floatval_mod(-5.0, -3.0) = -2.0,  expect -2.0

Changed 6 years ago by doughera

  Changed 5 years ago by reezer

Sorry for the late reply again. Of course I had to add '-lm' to the compiler flags to make it compile. I tried it with the ccflags I posted as part of the summary in my last (these were meant, right?) post and without. My output has always been:

floor( 5.0/-3.0) = -1.0, floatval_mod( 5.0, -3.0) =  2.0,  expect -1.0
floor(-5.0/ 3.0) = -1.0, floatval_mod(-5.0,  3.0) = -2.0,  expect  1.0
floor(-5.0/-3.0) =  1.0, floatval_mod(-5.0, -3.0) = -2.0,  expect -2.0

  Changed 5 years ago by reezer

Sorry for writing two posts, but if you want to offer you (doughera) a shell on this machine. bacek already got one and since it's nice for it to get a job I also want to offer an account to you. You could use it for any kind of testing/programming open source software not limited to this bug or parrot and use all it's resources.

Of course I can't guarantee for anything on this machine, but if you need any software installed or something similar I will do what I can. So, if you want to have an account write a mail with password or public key. You can find my contact information on:  http://reezer.freeshell.org/

Sorry for being off-topic

follow-up: ↓ 17   Changed 5 years ago by doughera

Thanks for the feedback. That settles it -- the floor() function on your system is simply broken. You should probably submit a bug report to OpenBSD with a simple test program, perhaps something trivial like:

#include <math.h>
#include <stdio.h>
int main(int argc, char **argv)
{
    double x = -1.5;
    printf("floor(%g) = %g, expected %g\n", x, floor(x), -2.0);
    return 0;
}

For Parrot, there are several possibilities:

1. Do nothing while waiting for an OpenBSD fix. The tests will continue to fail, accurately reflecting the broken floor() function.

2. Have parrot work around the problem. Everywhere that parrot currently calls "floor()", it could call Parrot_floor(). Configure.pl could write a simple test program (essentially the one I have above) and set a variable for "broken floor of a negative number." If the floor() function works correctly, then a simple #define Parrot_floor floor is all that's needed. Otherwise, in config/gen/platform/openbsd/math.c, there could be a replacement Parrot_floor() function that calculated it correctly for negative numbers.

in reply to: ↑ 16 ; follow-up: ↓ 18   Changed 5 years ago by reezer

Replying to doughera:

Thanks for the feedback. That settles it -- the floor() function on your system is simply broken. You should probably submit a bug report to OpenBSD with a simple test program <snip>

I reported it.

in reply to: ↑ 17   Changed 5 years ago by reezer

a fix has already been commited.

follow-up: ↓ 20   Changed 5 years ago by reezer

As I wrote the bug is fixed in OpenBSD, but this fix is not part of the stable branch yet. However, I guess it won't take long until it is.

I applied the patch to my system and ran the smoke test:

Test Summary Report
-------------------
t/op/arithmetics.t                        (Wstat: 0 Tests: 23 Failed: 0)
  TODO passed:   7
t/op/io.t                                 (Wstat: 0 Tests: 4 Failed: 0)
  TODO passed:   3-4
Files=397, Tests=11975, 1514 wallclock secs (20.83 usr 11.31 sys + 689.76 cusr 209.57 csys = 931.47 CPU)
Result: PASS

 http://smolder.plusthree.com/app/public_projects/report_details/24360

in reply to: ↑ 19   Changed 5 years ago by Infinoid

Replying to reezer:

t/op/arithmetics.t (Wstat: 0 Tests: 23 Failed: 0) TODO passed: 7

This test was TODOed specifically for OpenBSD. It sounds like we can remove that.

t/op/io.t (Wstat: 0 Tests: 4 Failed: 0) TODO passed: 3-4

Those tests are TODOed on everything except Linux, Win32 and Darwin. Looks like we can add OpenBSD to that list.

I've un-TODOed those tests in r39864. Thanks for the report!

follow-up: ↓ 22   Changed 5 years ago by bacek

Hello.

Can I close this ticket? Looks like main problems were resolved.

-- Bacek

in reply to: ↑ 21   Changed 5 years ago by jkeenan

Replying to bacek:

Hello. Can I close this ticket? Looks like main problems were resolved.

I believe we should close it and open new tickets for any related issues. (This thread is too long to continue to follow :-) )

kid51

  Changed 5 years ago by bacek

  • status changed from new to closed
  • resolution set to fixed

Resolving ticket. Original issue was resolved. Subsequent failures (if any) will be tracked in separate tickets.

Note: See TracTickets for help on using tickets.