This is an archive of the discontinued LLVM Phabricator instance.

[LLVM][MIPS] Fix different definition of off_t in LLDB and LLVM
ClosedPublic

Authored by nitesh.jain on Apr 17 2017, 6:25 AM.

Details

Summary

Fix is to define _LARGEFILE_SOURCE and _FILE_OFFSET_BITS=64. This fix will cause llvm::sys::fs::file_size() to return correct object size.

Diff Detail

Event Timeline

nitesh.jain created this revision.Apr 17 2017, 6:25 AM

Is this just GNU specific? BSD moved to 64-bit off_t on 32-bit platforms 20+ years ago.

It's perhaps no-op, but it might be noted in the commit message what platforms are supposed to be improved.

beanz edited edge metadata.Apr 17 2017, 10:54 AM

Can you please move this check into HandleLLVMOptions.cmake? By putting it into a module that is vended as part of LLVM's packaging then LLVM subprojects can have consistent settings when building out-of-tree.

This would enable @zturner's request to remove the duplicated code from LLDB.

Update diff as per suggestion.

Is this just GNU specific? BSD moved to 64-bit off_t on 32-bit platforms 20+ years ago.

It's perhaps no-op, but it might be noted in the commit message what platforms are supposed to be improved.

Yes.

beanz accepted this revision.Apr 18 2017, 1:25 PM

This looks good to me.

This revision is now accepted and ready to land.Apr 18 2017, 1:25 PM
emaste added a subscriber: emaste.Apr 21 2017, 6:40 PM

Is this just GNU specific? BSD moved to 64-bit off_t on 32-bit platforms 20+ years ago.

It's perhaps no-op, but it might be noted in the commit message what platforms are supposed to be improved.

I'd say it's better to put a comment in the source where it's set.

Is this just GNU specific? BSD moved to 64-bit off_t on 32-bit platforms 20+ years ago.

It's perhaps no-op, but it might be noted in the commit message what platforms are supposed to be improved.

I'd say it's better to put a comment in the source where it's set.

Added comment in the source.

This revision was automatically updated to reflect the committed changes.

Hi Nitesh,

this commit broke clang-cmake-mips. Can you investigate?

http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/3189

Thanks,
Simon

nitesh.jain added a comment.EditedApr 25 2017, 6:41 AM

Hi Nitesh,

this commit broke clang-cmake-mips. Can you investigate?

http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/3189

Thanks,
Simon

Hi Simon,

The assertion has been fixed in revision rL301307.

Thanks
Nitesh