OSSCA Chromium

기여 목록 / 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도 따로 관리하고 있음
  • 생각보다 많은걸 바꿔야 되는 수정이었고 테스트도 추가하여야 했는데, 리뷰어들이 새로 들어온 기여자임에도 리뷰를 잘 해줘서 수월하게 할 수 있었음

참고 자료