기여 목록 / 8349805
[date] Do not truncate getTimezoneOffset() to whole minutes
V8fixv8/v8Merged
문제 설명
- issue url: https://crbug.com/40814804
new Date().getTimezoneOffset()시에 소수(분수) 형태의 값이 나와야하는데 버림 이후 정수 형태의 분으로 표시되는 오류가 있었습니다ECMA262를 따르면, 해당 함수가 소수가 나와야 하지만 이를 따르지 않아 이를 이용하는 하위 라이브러리에서 사소한 오류가 발생하였음
해결 내용
JSDate::kTimezoneOffset에서 분이 아니라 밀리초를 저장하도록 바꿈DatePrototypeGetTimezoneOffset함수 내에서 이걸DateCache::kMsPerMin를 이용하여 분으로 변환하도록 바꿈
TNode<ExternalReference> isolate_ptr =
ExternalConstant(ExternalReference::isolate_address(isolate()));
TNode<Smi> field_index_smi = SmiConstant(field_index);
TNode<ExternalReference> function =
ExternalConstant(ExternalReference::get_date_field_function());
TNode<Object> result = CAST(
CallCFunction(function, MachineType::AnyTagged(),
std::make_pair(MachineType::Pointer(), isolate_ptr),
std::make_pair(MachineType::AnyTagged(), date_receiver),
std::make_pair(MachineType::AnyTagged(), field_index_smi)));
+ if (field_index == JSDate::kTimezoneOffset) {
+ // The offset comes back in milliseconds because the C function runs under
+ // DisallowGarbageCollection and cannot allocate the HeapNumber that a
+ // fractional-minute offset needs. Divide here instead. The cached path
+ // above would skip this, so the field must be uncached.
+ static_assert(JSDate::kTimezoneOffset >= JSDate::kFirstUncachedField);
+ Label if_nan(this, Label::kDeferred);
+ GotoIfNot(TaggedIsSmi(result), &if_nan);
+ TNode<Smi> offset_ms = CAST(result);
+ Return(ChangeFloat64ToTagged(Float64Div(
+ SmiToFloat64(offset_ms),
+ Float64Constant(static_cast<double>(DateCache::kMsPerMin)))));
+ BIND(&if_nan);
}
Return(result);
}발굴 과정
- 이슈 확인 중에 발견함
테스트 방법
별도의 테스트를 하나 더 추가하였습니다.
test/mjsunit/tzoffset-fractional-los-angeles.js:getTimezoneOffset()의 결과가 소수 형태로 나오는지 확인하는 테스트
배운 점
- V8은 별도의 레포지토리로 관리하여 AUTHORS도 따로 관리하고 있음
- 생각보다 많은걸 바꿔야 되는 수정이었고 테스트도 추가하여야 했는데, 리뷰어들이 새로 들어온 기여자임에도 리뷰를 잘 해줘서 수월하게 할 수 있었음