Skip to content

Commit a794611

Browse files
committed
Keep only the largest field as the public field
Without this, ffi-napi will iterate through the `fields` property and **sum up** the sizes of the member fields [1]. This will cause unions to appear to be larger than they are. This can cause memory corruption on 64-bit x86 Windows when the return type should be 8 bytes (and thus be returned in a register) but ffi-napi makes it into an indirect buffer instead. [1]: https://github.com/node-ffi-napi/node-ffi-napi/blob/1e7bbb170462f5f0880350cc4a518a2755b9337f/lib/type.js#L56
1 parent 858792b commit a794611

File tree

1 file changed

+21
-20
lines changed

1 file changed

+21
-20
lines changed

lib/union.js

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ function Union () {
6464

6565
UnionType.defineProperty = defineProperty
6666
UnionType.toString = toString
67-
UnionType.fields = {}
67+
UnionType.allFields = {}
6868

6969
// comply with ref's "type" interface
7070
UnionType.size = 0
@@ -73,7 +73,7 @@ function Union () {
7373
UnionType.get = get
7474
UnionType.set = set
7575

76-
// Read the fields list
76+
// Read the allFields list
7777
var arg = arguments[0]
7878
if (typeof arg === 'object') {
7979
Object.keys(arg).forEach(function (name) {
@@ -102,7 +102,7 @@ function set (buffer, offset, value) {
102102
var isUnion = value instanceof this
103103
if (isUnion) {
104104
// TODO: optimize - use Buffer#copy()
105-
Object.keys(this.fields).forEach(function (name) {
105+
Object.keys(this.allFields).forEach(function (name) {
106106
// hopefully hit the setters
107107
union[name] = value[name]
108108
})
@@ -151,7 +151,7 @@ function defineProperty (name, type) {
151151
var field = {
152152
type: type
153153
}
154-
this.fields[name] = field
154+
this.allFields[name] = field
155155

156156
// calculate the new size and alignment
157157
recalc(this);
@@ -168,33 +168,34 @@ function defineProperty (name, type) {
168168
}
169169

170170
function recalc (union) {
171-
// reset size and alignment
172-
union.size = 0
173-
union.alignment = 0
171+
var biggest
172+
var fieldNames = Object.keys(union.allFields)
174173

175-
var fieldNames = Object.keys(union.fields)
176-
177-
// loop through to set the size of the union of the largest member field
178-
// and the alignment to the requirements of the largest member
174+
// find the largest member field by size / alignment
179175
fieldNames.forEach(function (name) {
180-
var field = union.fields[name]
176+
var field = union.allFields[name]
181177
var type = field.type
182178

183179
var size = type.indirection === 1 ? type.size : ref.sizeof.pointer
184180
var alignment = type.alignment || ref.alignof.pointer
185181
if (type.indirection > 1) {
186182
alignment = ref.alignof.pointer
187183
}
188-
union.alignment = Math.max(union.alignment, alignment)
189-
union.size = Math.max(union.size, size)
184+
var fields = {}
185+
fields[name] = { type: type }
186+
187+
var current = { fields: fields, size: size, alignment: alignment }
188+
189+
if (!biggest || size > biggest.size || alignment > biggest.alignment) {
190+
biggest = current
191+
}
190192
})
191193

192-
// any padding
193-
var left = union.size % union.alignment
194-
if (left > 0) {
195-
debug('additional padding to the end of union:', union.alignment - left)
196-
union.size += union.alignment - left
197-
}
194+
union.alignment = biggest.alignment;
195+
union.size = biggest.size;
196+
// Pretend like we only have this one field so that
197+
// ffi-napi doesn't overcount the size
198+
union.fields = biggest.fields;
198199
}
199200

200201

0 commit comments

Comments
 (0)