Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
base fork: stevedekorte/io
base: 7bf702cf3a
...
head fork: stevedekorte/io
compare: 91c356f14e
  • 2 commits
  • 1 file changed
  • 6 commit comments
  • 2 contributors
Commits on Oct 12, 2012
@jeremyevans jeremyevans Ensure 16-byte alignment for amd64 coroutine stack
The AMD64 ABI requires that stacks be 16-byte aligned.  stackend is
going to be 16-byte aligned, 128 is a multiple of 16, and
sizeof(uintptr_t) is going to 8 on amd64, so this results in a stack
that is not 16-byte aligned on amd64 (at least for platforms where
this code branch is used, such as OpenBSD).  This results in bus errors
when floating point numbers are converted to strings, since snprintf
calls movaps with a non 16-byte aligned memory location.

With this patch, the iovm correctness tests pass on OpenBSD/amd64.
84c9e4f
@stevedekorte Merge pull request #230 from jeremyevans/patch-1
Ensure 16-byte alignment for amd64 coroutine stack
91c356f
Showing with 1 addition and 1 deletion.
  1. +1 −1  libs/coroutine/source/Coro.c
View
2  libs/coroutine/source/Coro.c
@@ -599,7 +599,7 @@ void Coro_setup(Coro *self, void *arg)
if (64 > (- sav[i] + (uintptr_t)&i))
break;
assert(i < sz);
- sav[i] = stackend - sizeof(uintptr_t) - 128;
+ sav[i] = stackend - sizeof(uintptr_t)*2 - 128;
}
}

Showing you all comments on commits in this comparison.

@jeremytregunna

The proper way to do alignment is like this:

static inline size_t aligned_size(size_t size, size_t alignment)
{
    assert(alignment <= 0x8000);
    size_t r = size + --alignment + 2;
    return (r + 2 + alignment) & ~alignment;
}

Calling this function assumes that "alignment" is power of 2, and "size" would be the original stackend - sizeof(uintptr_t) - 128.

@jeremyevans

That's certainly looks like a more portable solution, compared to the quick hack I used. I'll be happy as long as the problem get's fixed. Not being familiar with the amd64 ABI or Io's internals, it took me many hours to diagnose the underlying problem causing the bus error, and figure out where the fix should be applied.

@stevedekorte

I'll accept whichever working version you guys recommend. Thanks for helping with this.

@jeremyevans

If you plan on having Io run on many platforms with different alignment requirements, then Jeremy Tregunna's code is probably best. If you only care about fixing amd64, then this pull request gets the job done with the least number of changes (2 characters).

@jeremytregunna

I'll test out my code in place, and see if there are any regressions. Should all look ok, then I will send another pull.

@jeremytregunna

Alright tested, and seems to work just fine. It's in #232.

Something went wrong with that request. Please try again.