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

TTMath broken on Windows: GCC ASM stack corruption, VCC refuses inline ASM, string conversion #14

Closed
mratsim opened this issue Apr 3, 2018 · 3 comments

Comments

@mratsim
Copy link
Contributor

mratsim commented Apr 3, 2018

From @coffeepots

VCC doesn't support inline asm: https://www.ttmath.org/forum/how_to_compile_with_visual_studio_2010_64-bit

GCC 6.3 on windows has a stack corruption issue:

Before #11 it was this:

#0  ttmath::UInt<8ul>::Pow (this=0x65fd70, pow=...) at c:/Status/nim-ttmath/src/headers/ttmathuint.h:2418
#1  0x0000000000401603 in pow (a=0x40a040 <a>, b=0x40a060 <b>) at c:\Status\nim-ttmath\src\headers\test.cpp:12
#2  0x0000000000401722 in main () at c:\Status\nim-ttmath\src\headers\test.cpp:21

After it is now this:

#0  ttmath::UInt<8ul>::Pow (this=0x65fd70, pow=...) at c:/Status/nim-ttmath/src/headers/ttmathuint.h:2419
#1  0x0000000000401603 in pow (a=0x40a040 <a>, b=0x40a060 <b>) at c:\Status\nim-ttmath\src\headers\test.cpp:12
#2  0x0000000000200b91 in ?? ()

2418-2419 corresponds to the Rcr2_one call in the following part

	uint Pow(UInt<value_size> pow)
	{
		if(pow.IsZero() && IsZero())
			// we don't define zero^zero
			return 2;

		UInt<value_size> start(*this);
		UInt<value_size> result;
		result.SetOne();
		uint c = 0;

		while( !c )
		{
			if( pow.table[0] & 1 )
				c += result.Mul(start);

			pow.Rcr2_one(0);
			if( pow.IsZero() )
				break;

			c += start.Mul(start);
		}

		*this = result;

		TTMATH_LOGC("UInt::Pow(UInt<>)", c)

	return (c==0)? 0 : 1;
	}
	template<uint value_size>
	uint UInt<value_size>::Rcr2_one(uint c)
	{
	sint b = value_size;
	uint * p1 = table;
	

		#ifndef __GNUC__
			c = ttmath_rcr_x64(p1,b,c);
		#endif


		#ifdef __GNUC__
		uint dummy;

		__asm__  __volatile__(

			"negq %%rax						\n"   // CF=1 if rax!=0 , CF=0 if rax==0

		"1:									\n"
			"rcrq $1, -8(%%rbx, %%rcx, 8)	\n"

			"decq %%rcx						\n"
		"jnz 1b								\n"

			"adcq %%rcx, %%rcx				\n"

			: "=c" (c), "=a" (dummy)
			: "0" (b),  "1" (c), "b" (p1)
			: "cc", "memory" );

		#endif

		TTMATH_LOGC("UInt::Rcr2_one", c)
@mratsim
Copy link
Contributor Author

mratsim commented Apr 3, 2018

A simple fix would be to deactivate ASM on Windows. In any case we target mobile and restricted devices (ARM and MIPS) and no ASM is available for those.

when defined(windows):
  {.passC: "-DTTMATH_NOASM".}

@mratsim mratsim changed the title TTMath broken on Windows: GCC ASM stack corruption, VCC refuses ASM TTMath broken on Windows: GCC ASM stack corruption, VCC refuses inline ASM, string conversion Apr 3, 2018
@mratsim
Copy link
Contributor Author

mratsim commented Apr 3, 2018

Adding to the list of complaints: string conversion doesn't work on Windows: https://ci.appveyor.com/project/jarradh/nim-ttmath/build/22/job/yjwudram3m15we4f#L1378

[Suite] ttmath
    test1.nim(9, 15): Check failed: $(a + b) == "28497092031222200795837"
    $(a + b) was 25873220285
    test1.nim(10, 12): Check failed: a + b == "28497092031222200795837".i256
    a + b was 25873220285
    "28497092031222200795837".i256 was 103416509
    test1.nim(12, 12): Check failed: a + b < "28497092031222200795838".i256
    a + b was 25873220285
    "28497092031222200795838".i256 was 103416510
    test1.nim(13, 9): Check failed: -a == "-12345678910111213141516".i256
    -a was 12309411850224945125033585585165461537827125511483805057073903131095380176159220
    "-12345678910111213141516".i256 was 848018956
  [FAILED] Ints
    test1.nim(20, 15): Check failed: $(a + b) == "28497092031222200795837"
    $(a + b) was 25873220285
    test1.nim(21, 12): Check failed: a + b == "28497092031222200795837".u256
    a + b was 25873220285
    "28497092031222200795837".u256 was 103416509
    test1.nim(23, 12): Check failed: a + b < "28497092031222200795838".u256
    a + b was 25873220285
    "28497092031222200795838".u256 was 103416510
  [FAILED] UInts
[Suite] Testing conversion functions: Hex, Bytes, Endianness
  [OK] hex -> uint256
    test1.nim(35, 26): Check failed: SECPK1_N.toHex == SECPK1_N_HEX
    SECPK1_N.toHex was d0364141
    SECPK1_N_HEX was fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141
  [FAILED] uint256 -> hex
    test1.nim(38, 43): Check failed: readUIntBE[256](SECPK1_N_BYTES) == SECPK1_N
    readUIntBE[256](SECPK1_N_BYTES) was 103443464614620605021057432097659110162469981302141632742275809279
    SECPK1_N was 3493216577
  [FAILED] hex -> big-endian array -> uint256
    test1.nim(41, 34): Check failed: SECPK1_N.toByteArrayBE == SECPK1_N_BYTES
    SECPK1_N.toByteArrayBE was [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 208, 54, 65, 65]
    SECPK1_N_BYTES was [255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 254, 186, 174, 220, 230, 175, 72, 160, 59, 191, 210, 94, 140, 208, 54, 65, 65]
  [FAILED] uint256 -> big-endian array -> hex
[Suite] Confirming consistency: hex vs decimal conversion
  [OK] Alice signature
  [OK] Bob signature
  [OK] Eve signature

@mratsim
Copy link
Contributor Author

mratsim commented Apr 3, 2018

Fixed by @coffeepots 38b1ca4

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

1 participant