Euphoria Ticket #818: functions long integer C-calls are not handled on Linux-32

There are many other problems. It is possible the C-only code in be_callc.c will return values which are not valid objects when the underlying C code returns some very negative values with some types.

Details

Type: Bug Report Severity: Major Category: Interpreter
Assigned To: SDPringle Status: Fixed Reported Release: 4.1.0
Fixed in SVN #: View VCS: none Milestone: 4.1.0

1. Comment by SDPringle Nov 07, 2012

There is a nomenclature with the constants C_XXX. The last byte is the width of the data type, the first nibble describes if it is signed, unsigned or other (like floating point). This changed in 4.1, so that C_LONG broke from this convention, so we need to handle this case special or change it back to follow the convention. Although in 64-bit this will mean it will be different values in 32 vs. 64 bit.

2. Comment by mattlewis Nov 07, 2012

I don't think the original convention works once 64-bits shows up, since sizes change across operating systems and architectures. I think we just need to handle it as a special case.

3. Comment by SDPringle Nov 07, 2012

See: hg:euphoria/rev/483f3f49b669

changeset: 5820:483f3f49b669 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Nov 07 13:28:51 2012 -0300 files: source/Makefile.gnu tests/t_callc.e description:

  • tests for returned values
  • ticket 818

4. Comment by mattlewis Nov 07, 2012

The test tries to use "lib818.dll". What is this? This is not a good test.

5. Comment by SDPringle Nov 07, 2012

See: hg:euphoria/rev/183b4d931812

changeset: 5821:183b4d931812 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Nov 07 13:58:49 2012 -0300 files: tests/t_callc.e description:

  • aligned test for better readability
  • ticket 818

6. Comment by SDPringle Nov 08, 2012

I am thinking some more trivial C functions can help this along for testing argument passing to C functions: 

// return true iff the argument is minus two billion 
bool is_C_DOUBLE_m2billion(double d) 
bool is_C_FLOAT_m2billion(float f) 
bool is_C_INT_m2billion(int i) 
// etc...

7. Comment by mattlewis Nov 08, 2012

I've updated the test and the build process for the testing library, and I think that the C_LONG and C_LONLONGs are working correctly on 64 and 32 bit linux.

However, the callback test for C_LONGLONG fails on 32 bit linux. I'm not sure if this is correct or not, since the function is actually returning a 32-bit value, but we're expecting a 64-bit value.

hg:euphoria/rev/b8a137a821aa

8. Comment by SDPringle Nov 08, 2012

The value MININT, is not supposed to be twenty less than the standard MININT, it is supposed to be twenty less than our EUPHORIA MININT, which is still a negative value in C's intptr_t's but too negative to be stored in a EUPHORIA integer.

9. Comment by mattlewis Nov 08, 2012

Testing on both 64 and 32 bit architectures complicates things, because you can fit all int32_t values inside a euphoria integer on a 64-bit platform and long ints vary by operating system.

Regarding my updates:

  • C_INT: 0xc0000000 (32-bit euphoria MININT value) is hard coded, because that's the only value that really matters for this type on any system
  • C_LONG: value is based on the architecture, since longs are different on different 64-bit operating systems. Note that all C_LONG values fit into a euphoria integer on 64-bit Windows.
  • C_LONGLONG: uses the MININT value from the euphoria.h header. On 32-bit and 64-bit systems, we always get a value that can't fit in an integer.

10. Comment by SDPringle Nov 09, 2012

See: hg:euphoria/rev/019b93c7ea8b

changeset: 5827:019b93c7ea8b parent: 5821:183b4d931812 user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Nov 07 20:33:14 2012 -0300 files: source/test818.c description:

  • made C code clearer
  • ticket 818

11. Comment by SDPringle Nov 10, 2012

See: hg:euphoria/rev/6bdc2ef9801d

changeset: 5831:6bdc2ef9801d tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Sat Nov 10 11:40:14 2012 -0300 files: source/test818.c tests/t_callc.e description:

  • renamed functions in test818.c and t_callc.e
  • ticket 818

12. Comment by SDPringle Nov 10, 2012

See: hg:euphoria/rev/843d39f67e2a

changeset: 5832:843d39f67e2a tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Sun Nov 11 00:22:54 2012 -0300 files: source/test818.c tests/t_callc.e description:

  • added support for and tests for returning values from C functions that:
    • return NOVALUE.
    • return values beyond the EUPHORIA's MAXINT.
    • return values inside EUPHROIA'S integer range on both the positive and negative side.
  • ticket 818

13. Comment by SDPringle Nov 12, 2012

See: hg:euphoria/rev/d66aa2c1da08

changeset: 5833:d66aa2c1da08 user: Shawn Pringle <shawn.pringle@gmail.com> date: Mon Nov 12 09:47:54 2012 -0300 files: tests/t_callc.e description:

  • corrected a conjunction (and not or)
  • corrected char test, they are signed characters
  • ticket 818

14. Comment by SDPringle Nov 12, 2012

See: hg:euphoria/rev/60bda582c56c

changeset: 5836:60bda582c56c tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Mon Nov 12 15:34:24 2012 -0300 files: tests/t_callc.e description:

  • added tests for sending values to and receiving them back the same for integer types.
  • with the functions that only return values this tests that things are being passed to C well.
  • ticket 818

15. Comment by SDPringle Nov 12, 2012

See: hg:euphoria/rev/d0b6c19332a3

changeset: 5837:d0b6c19332a3 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Mon Nov 12 16:36:15 2012 -0300 files: source/be_callc.c tests/t_callc.e description:

  • added test for testing for some simple but large numbers with long longs
  • ticket 818

16. Comment by mattlewis Nov 12, 2012

See: hg:euphoria/rev/c4cf83a3db32

changeset: 5838:c4cf83a3db32 tag: tip user: Matt Lewis date: Mon Nov 12 20:38:17 2012 -0500 files: source/be_callc.c source/test818.c description:

  • fixed C-call return checks to be portable
  • line endings for test818.c ?
  • ticket 818

17. Comment by SDPringle Nov 13, 2012

See: hg:euphoria/rev/ca064d0b6d3f

changeset: 5841:ca064d0b6d3f parent: 5833:d66aa2c1da08 user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 13 09:18:34 2012 -0300 files: source/test818.c description:

  • made MAKE_BORDER_FUNCTIONS more readble
  • created MAKE_ID_FUNCTION macro.
  • created id functions
  • created bit_repeat function in test818.c
  • this is for a bad commit message changeset
  • ticket 818

18. Comment by SDPringle Nov 13, 2012

See: hg:euphoria/rev/c5549ba58c85

changeset: 5843:c5549ba58c85 tag: tip parent: 5842:cb7b75ebca1b parent: 5837:d0b6c19332a3 user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 13 09:39:33 2012 -0300 description:

  • make C+asm and C only use the same code for interfacing with the EUPHORIA arguments passed and converting C to EUPHORIA.
  • ticket 818

19. Comment by SDPringle Nov 13, 2012

See: hg:euphoria/rev/b9e728e0e199

changeset: 5844:b9e728e0e199 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 13 10:09:51 2012 -0300 files: tests/t_callc.e description:

  • removed unnecessary 4.0 compatibility lines
  • ticket 818

20. Comment by mattlewis Nov 13, 2012

See: hg:euphoria/rev/aac242059778

changeset: 5845:aac242059778 user: Matt Lewis date: Tue Nov 13 09:17:13 2012 -0500 files: source/be_callc.c source/test818.c description:

  • re-ported call-c code
  • line endings again on test818.c?
  • ticket 818

21. Comment by SDPringle Nov 13, 2012

See: hg:euphoria/rev/22afc58e8786

changeset: 5848:22afc58e8786 user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 13 15:19:05 2012 -0300 files: tests/t_callc.e description:

  • renamed tests
  • added comments
  • ticket 818

22. Comment by SDPringle Nov 13, 2012

See: hg:euphoria/rev/651d69fe80ba

changeset: 5850:651d69fe80ba tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 13 16:21:01 2012 -0300 files: source/be_callc.c description:

  • moved macro PUSH_INT64_ARG to above the function defintion
  • ticket 818

23. Comment by mattlewis Nov 14, 2012

See: hg:euphoria/rev/7a9047b505ed

changeset: 5851:7a9047b505ed parent: 5847:834a071f0089 user: Matt Lewis date: Wed Nov 14 06:40:42 2012 -0500 files: source/test818.c description:

  • ifdef out border int / long border functions where appropriate
  • ticket 818

24. Comment by mattlewis Nov 14, 2012

See: hg:euphoria/rev/0d945c0e414f

changeset: 5852:0d945c0e414f tag: tip parent: 5851:7a9047b505ed parent: 5850:651d69fe80ba user: Matt Lewis date: Wed Nov 14 15:17:35 2012 -0500 files: source/Makefile.gnu source/be_callc.c source/be_socket.c source/test818.c tests/t_callc.e description:

  • fix lib818.dll build to use $(CC)
  • skip long long callbacks on 32-bit architectures, since that cannot work
  • fixed long long parameter passing for 32-bit architectures
  • fixed some type casting issues with C return types
  • ticket 818

25. Comment by SDPringle Nov 20, 2012

Putting '+' in define_c_func without making them cdecl in the source file is a recipe for crashing the test. Apart from that, the C+assembler doesn't work with MINGW.

26. Comment by SDPringle Nov 20, 2012

See: hg:euphoria/rev/443b22390072

changeset: 5861:443b22390072 user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 20 17:56:25 2012 -0300 files: source/test818.c tests/t_callc.e description:

  • added a test that calls a routine that passes ten arguments and returns a double
  • returned functions to use default calling convention in the test.
  • ticket 818

27. Comment by SDPringle Nov 20, 2012

See: hg:euphoria/rev/ae9253240aa3

changeset: 5862:ae9253240aa3 user: Shawn Pringle <shawn.pringle@gmail.com> date: Tue Nov 20 18:26:58 2012 -0300 files: source/be_callc.c description:

  • made MinGW use the C-only version of call_routine. Upgraded C-only logic to handle long longs.
  • ticket 818

28. Comment by mattlewis Nov 20, 2012

I don't understand the latest changes:

  • At least with MinGW 4.6.3, the ASM callc code works just fine.
  • None of the functions in test818.c are declared as stdcall. Therefore, we should define them properly using the default calling convention on 32-bit windows: cdecl.

29. Comment by mattlewis Nov 20, 2012

See: hg:euphoria/rev/782d4ffdbb8b

changeset: 5873:782d4ffdbb8b tag: tip user: Matt Lewis date: Tue Nov 20 17:18:07 2012 -0500 files: source/be_callc.c tests/t_callc.e description:

  • reenable ASM on 32-bit Windows built with MinGW
  • fix calling conventions in t_callc.e
  • ticket 818

30. Comment by SDPringle Nov 22, 2012

See: hg:euphoria/rev/dcc152ad59f8

changeset: 5877:dcc152ad59f8 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Thu Nov 22 09:47:28 2012 -0300 files: source/test818.c tests/t_callc.e description:

  • added some tests for numbers with large negative values
  • ticket 818

31. Comment by mattlewis Nov 28, 2012

See: hg:euphoria/rev/d7045090ca1f

changeset: 5882:d7045090ca1f user: Matt Lewis date: Wed Nov 28 09:00:55 2012 -0500 files: source/be_callc.c tests/t_callc.e description:

  • fix C_LONGLONG returns for 64-bits
  • t_callc.e fixes:
  • remove errant calling convention when defining C variable
  • use floor() to prevent double / long double conversion issues from causing false positive
  • ticket 818

32. Comment by mattlewis Dec 30, 2012

The commit comment was incorrect, but hg:euphoria/rev/fbb9af625332 fixes E_* return types for 64-bit euphoria.

33. Comment by jimcbrown Dec 30, 2012

This is still open? I.E. it's fixed for 64bit but we need a 32bit fix also? What's left to do?

34. Comment by SDPringle Jan 13, 2013

We need C to call external APIs this is at least Major in severity.

35. Comment by jimcbrown Jan 13, 2013

What currently works and what is currently broken? The original description is good, but from the previous comments (and the work that was done), this is now unclear.

36. Comment by SDPringle Jan 14, 2013

What is left to do is more testing: more tests that include floating point types. Especially, since in 64-bits the first ints and floats go into special registers. If you exhaust all of your float registers, will your ints no longer go into the registers they should go to?

37. Comment by jimcbrown Jan 14, 2013

You mean more (and new) tests need to be added? Or just more people need to run the existing tests that you have written?

38. Comment by SDPringle Jan 14, 2013

See: hg:euphoria/rev/68ac318bc330

changeset: 5923:68ac318bc330 user: Shawn Pringle <shawn.pringle@gmail.com> date: Sun Jan 13 11:45:35 2013 -0300 files: source/test818.c tests/t_callc.e description:

  • added test for 8 doubles and two long longs
  • ticket 818

39. Comment by mattlewis Jan 14, 2013

I'm getting the following failures on Win64: 

  failed: Can call and things are passed correctly for ten argument functions, 
          expected: 1125899930950272 
           but got: 1125899919210882 
  failed: Testing passing eight doubles only, 
          expected: 4.88502064518755e+157 
           but got: 10430452.265625 
  failed: Testing passing eight doubles only and two long long ints, 
          expected: -0.692138671875 
           but got: 354.375 
  87 tests run, 84 passed, 3 failed, 97% success

So...more work to do with the Windows x64 calling convention.

40. Comment by mattlewis Jan 15, 2013

See: hg:euphoria/rev/b83d42d5a9cd

changeset: 5931:b83d42d5a9cd tag: tip user: Matt Lewis date: Tue Jan 15 10:14:34 2013 -0500 files: source/be_callc.c description:

  • Win64 call C fixes
  • ticket 818

41. Comment by SDPringle Jan 16, 2013

See: hg:euphoria/rev/0a3e1b6a35ea

changeset: 5932:0a3e1b6a35ea parent: 5925:84d9b070148b user: Shawn Pringle <shawn.pringle@gmail.com> date: Mon Jan 14 14:29:37 2013 -0300 files: tests/t_callc.e description:

  • corrected a test implementation
  • ticket 818

42. Comment by mattlewis Mar 01, 2013

I think we're pretty good here. I've marked it as "Fixed, Please Confirm." Anyone disagree?

43. Comment by SDPringle Mar 02, 2013

Good thing. I came up with the last test by looking carefully at the source in be_callc. The comments say that the first 5 floating point arguments go into registers but then the macro is assigned to 8. Is this comment wrong?

There should be another test that passes 8 longs or 8 long longs and then doubles or floats after that.

44. Comment by mattlewis Mar 04, 2013

See: hg:euphoria/rev/f166ae3a00a2

changeset: 6019:f166ae3a00a2 tag: tip user: Matt Lewis date: Mon Mar 04 06:18:09 2013 -0500 files: source/be_callc.c source/test818.c tests/t_callc.e description:

  • fixed be_callc comment regarding FP registers in AMD64 calling convention
  • added test for 8 longs, 6 doubles C-call
  • fixed test818.c to export variables on Windows
  • fixed test for exported variables
  • ticket 818

45. Comment by mattlewis Apr 02, 2013

hg:euphoria/rev/01599c2848b7 made some fixes for ARM in hardfloat mode. I'm not sure about the state of softfloat.

t_callc.e and t_dep.e now pass.

46. Comment by SDPringle Aug 29, 2013

See: hg:euphoria/rev/1fc0cfc1f78c

changeset: 6159:1fc0cfc1f78c tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Thu Aug 29 09:52:43 2013 -0300 files: source/test818.c tests/t_callc.e description:

  • adding tests using floats and doubles together
  • ticket 818

47. Comment by SDPringle Aug 29, 2013

See: hg:euphoria/rev/f8f142774509

changeset: 6160:f8f142774509 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Thu Aug 29 09:58:48 2013 -0300 files: tests/t_callc.e description:

  • added comments, removed blank line
  • ticket 818

48. Comment by SDPringle Dec 25, 2014

I am confirming this myself because nobody else will. The tests all pass.

Search



Quick Links

User menu

Not signed in.

Misc Menu